[RFT][PATCH 0/4] ALSA: dice: enabled to handle several streams

[RFT][PATCH 0/4] ALSA: dice: enabled to handle several streams

[Takashi Sakamoto]

Hi,

This patchset adds support for whole available isochronous streams to ALSA
dice driver. As a result, for some models, additional PCM substreams are
available via subdevice of ALSA PCM character device. In configuration space
of alsa-lib, it's represented like 'hw:0,0,1'(card/device/subdevice).

It's soon in the last week of this developing cycle, while I hope to finish
my work for ALSA dice driver by this patchset. Unfortunately, this patchset
adds huge changes and includes possibilities of regression. I'm happy to
receive your reports to test with any of your Dice units.

For testers, I push whole patches to 'dice-several-streams' branch in my
private repository. The changed driver is available from this branch. I
recommend to use DKMS to maintain the driver, instead of installing by your
own.
https://github.com/takaswie/snd-firewire-improve/tree/dice-several-streams

To check whether your units support several isochronous streams, please
read output from /proc/asound/cardX/dice. When seeing 'tx1' (input) or
'rx1' (output), the additional subdevices are available. For example,

$ arecord -l
...
card 1: Pro26000f5a [Pro26-000f5a], device 0: DICE [Pro26-000f5a]
  Subdevices: 2/2
  Subdevice #0: subdevice #0
  Subdevice #1: subdevice #1

In above case, you can use two subdevices for PCM capture, thus:
 * arecord -D plughw:1,0,0 file1.wav
 * arecord -D plughw:1,0,1 file2.wav

In the middle of next week, I plan to post this patchset again for
merging. So it's nice to test till then.


Regards

Takashi Sakamoto (4):
  ALSA: dice: have two sets of isochronous resources/streams
  ALSA: dice: handle whole available isochronous streams
  ALSA: dice: handle several PCM substreams when any isochronous streams
    are available
  ALSA: dice: force to add two pcm devices for listed models

 sound/firewire/dice/dice-midi.c   |   8 +-
 sound/firewire/dice/dice-pcm.c    | 110 +++++++----
 sound/firewire/dice/dice-stream.c | 400 ++++++++++++++++++++++++--------------
 sound/firewire/dice/dice.c        |  41 ++++
 sound/firewire/dice/dice.h        |  33 +++-
 5 files changed, 401 insertions(+), 191 deletions(-)

-- 
2.7.0

[PATCH 1/4] ALSA: dice: have two sets of isochronous resources/streams

[Takashi Sakamoto]

Currently ALSA dice driver handles a pair of isochronous resources for
IEC 61883-1/6 packet streaming. While, according to some documents about
ASICs named as 'Dice', several isochronous streams are available.

Here, I start to describe ASICs produced under 'Dice' name.
 * Dice II (designed by wavefront semiconductor, including TCAT's IP)
   * STD (with limited functionality of DTCP)
   * CP  (with full functionality of DTCP)
 * TCD2210/2210-E (so-called 'Dice Mini')
 * TCD2220/2220-E (so-called 'Dice Jr.')
 * TCD3070-CH (so-called 'Dice III')

Some documents are public and we can see hardware design of them. We can
find some articles about hardware internal register definitions
(not registers exported to IEEE 1394 bus).

* DICE II User Guide
  * http://www.tctechnologies.tc/archive/downloads/dice_ii_user_guide.pdf
    * 6.1 AVS Audio Receivers
      * Table 6.1: AVS Audio Receiver Memory Map
        * ARX1-ARX4
    * 6.2 AVS Audio Transmitters
      * Table 6.2: AVS Audio Transmitter Memory Map
        * ATX1, ATX2
* TCD22xx User Guide
  * http://www.tctechnologies.tc/downloads/tcd22xx_user_guide.pdf
    * 6.1 AVS Audio Receivers
      * Table 66: AVS Audio Receiver Memory Map
        * ARX1, ARX2
    * 6/2 AVS Audio Transmitters
      * Table 67: AVS Audio Transmitter Memory Map
        * ATX1, ATX2
* DICE III
  * http://www.tctechnologies.tc/downloads/TCD3070-CH.pdf
    * Dual stream 63 channel transmitter/receiver

For Dice II and TCD22xx series, maximum 16 data channels are transferred in
an AMDTP packet, while for Dice III, maximum 32 data channels are
transferred.

According to the design of the series of these ASICs, this commit allows
this driver to handle additional set of isochronous resources. For
practical reason, two pair of isochronous resources are added. As of this
commit, this driver still use a pair of the first isochronous resources.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/dice/dice-midi.c   |  8 +++---
 sound/firewire/dice/dice-pcm.c    | 34 ++++++++++++++----------
 sound/firewire/dice/dice-stream.c | 54 +++++++++++++++++++--------------------
 sound/firewire/dice/dice.h        | 31 +++++++++++++++++++---
 4 files changed, 79 insertions(+), 48 deletions(-)

diff --git a/sound/firewire/dice/dice-midi.c b/sound/firewire/dice/dice-midi.c
index 2461311..a040617 100644
--- a/sound/firewire/dice/dice-midi.c
+++ b/sound/firewire/dice/dice-midi.c
@@ -52,10 +52,10 @@ static void midi_capture_trigger(struct snd_rawmidi_substream *substrm, int up)
 	spin_lock_irqsave(&dice->lock, flags);
 
 	if (up)
-		amdtp_am824_midi_trigger(&dice->tx_stream,
+		amdtp_am824_midi_trigger(&dice->tx_stream[0],
 					  substrm->number, substrm);
 	else
-		amdtp_am824_midi_trigger(&dice->tx_stream,
+		amdtp_am824_midi_trigger(&dice->tx_stream[0],
 					  substrm->number, NULL);
 
 	spin_unlock_irqrestore(&dice->lock, flags);
@@ -69,10 +69,10 @@ static void midi_playback_trigger(struct snd_rawmidi_substream *substrm, int up)
 	spin_lock_irqsave(&dice->lock, flags);
 
 	if (up)
-		amdtp_am824_midi_trigger(&dice->rx_stream,
+		amdtp_am824_midi_trigger(&dice->rx_stream[0],
 					 substrm->number, substrm);
 	else
-		amdtp_am824_midi_trigger(&dice->rx_stream,
+		amdtp_am824_midi_trigger(&dice->rx_stream[0],
 					 substrm->number, NULL);
 
 	spin_unlock_irqrestore(&dice->lock, flags);
diff --git a/sound/firewire/dice/dice-pcm.c b/sound/firewire/dice/dice-pcm.c
index a5c9b58..e252949 100644
--- a/sound/firewire/dice/dice-pcm.c
+++ b/sound/firewire/dice/dice-pcm.c
@@ -22,7 +22,7 @@ static int limit_channels_and_rates(struct snd_dice *dice,
 	 * Retrieve current Multi Bit Linear Audio data channel and limit to
 	 * it.
 	 */
-	if (stream == &dice->tx_stream) {
+	if (stream == &dice->tx_stream[0]) {
 		err = snd_dice_transaction_read_tx(dice, TX_NUMBER_AUDIO,
 						   reg, sizeof(reg));
 	} else {
@@ -74,10 +74,10 @@ static int init_hw_info(struct snd_dice *dice,
 
 	if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) {
 		hw->formats = AM824_IN_PCM_FORMAT_BITS;
-		stream = &dice->tx_stream;
+		stream = &dice->tx_stream[0];
 	} else {
 		hw->formats = AM824_OUT_PCM_FORMAT_BITS;
-		stream = &dice->rx_stream;
+		stream = &dice->rx_stream[0];
 	}
 
 	err = limit_channels_and_rates(dice, runtime, stream);
@@ -122,6 +122,7 @@ static int capture_hw_params(struct snd_pcm_substream *substream,
 			     struct snd_pcm_hw_params *hw_params)
 {
 	struct snd_dice *dice = substream->private_data;
+	struct amdtp_stream *stream = &dice->tx_stream[0];
 	int err;
 
 	err = snd_pcm_lib_alloc_vmalloc_buffer(substream,
@@ -135,7 +136,7 @@ static int capture_hw_params(struct snd_pcm_substream *substream,
 		mutex_unlock(&dice->mutex);
 	}
 
-	amdtp_am824_set_pcm_format(&dice->tx_stream, params_format(hw_params));
+	amdtp_am824_set_pcm_format(stream, params_format(hw_params));
 
 	return 0;
 }
@@ -143,6 +144,7 @@ static int playback_hw_params(struct snd_pcm_substream *substream,
 			      struct snd_pcm_hw_params *hw_params)
 {
 	struct snd_dice *dice = substream->private_data;
+	struct amdtp_stream *stream = &dice->rx_stream[0];
 	int err;
 
 	err = snd_pcm_lib_alloc_vmalloc_buffer(substream,
@@ -156,7 +158,7 @@ static int playback_hw_params(struct snd_pcm_substream *substream,
 		mutex_unlock(&dice->mutex);
 	}
 
-	amdtp_am824_set_pcm_format(&dice->rx_stream, params_format(hw_params));
+	amdtp_am824_set_pcm_format(stream, params_format(hw_params));
 
 	return 0;
 }
@@ -196,26 +198,28 @@ static int playback_hw_free(struct snd_pcm_substream *substream)
 static int capture_prepare(struct snd_pcm_substream *substream)
 {
 	struct snd_dice *dice = substream->private_data;
+	struct amdtp_stream *stream = &dice->tx_stream[0];
 	int err;
 
 	mutex_lock(&dice->mutex);
 	err = snd_dice_stream_start_duplex(dice, substream->runtime->rate);
 	mutex_unlock(&dice->mutex);
 	if (err >= 0)
-		amdtp_stream_pcm_prepare(&dice->tx_stream);
+		amdtp_stream_pcm_prepare(stream);
 
 	return 0;
 }
 static int playback_prepare(struct snd_pcm_substream *substream)
 {
 	struct snd_dice *dice = substream->private_data;
+	struct amdtp_stream *stream = &dice->rx_stream[0];
 	int err;
 
 	mutex_lock(&dice->mutex);
 	err = snd_dice_stream_start_duplex(dice, substream->runtime->rate);
 	mutex_unlock(&dice->mutex);
 	if (err >= 0)
-		amdtp_stream_pcm_prepare(&dice->rx_stream);
+		amdtp_stream_pcm_prepare(stream);
 
 	return err;
 }
@@ -223,13 +227,14 @@ static int playback_prepare(struct snd_pcm_substream *substream)
 static int capture_trigger(struct snd_pcm_substream *substream, int cmd)
 {
 	struct snd_dice *dice = substream->private_data;
+	struct amdtp_stream *stream = &dice->tx_stream[0];
 
 	switch (cmd) {
 	case SNDRV_PCM_TRIGGER_START:
-		amdtp_stream_pcm_trigger(&dice->tx_stream, substream);
+		amdtp_stream_pcm_trigger(stream, substream);
 		break;
 	case SNDRV_PCM_TRIGGER_STOP:
-		amdtp_stream_pcm_trigger(&dice->tx_stream, NULL);
+		amdtp_stream_pcm_trigger(stream, NULL);
 		break;
 	default:
 		return -EINVAL;
@@ -240,13 +245,14 @@ static int capture_trigger(struct snd_pcm_substream *substream, int cmd)
 static int playback_trigger(struct snd_pcm_substream *substream, int cmd)
 {
 	struct snd_dice *dice = substream->private_data;
+	struct amdtp_stream *stream = &dice->rx_stream[0];
 
 	switch (cmd) {
 	case SNDRV_PCM_TRIGGER_START:
-		amdtp_stream_pcm_trigger(&dice->rx_stream, substream);
+		amdtp_stream_pcm_trigger(stream, substream);
 		break;
 	case SNDRV_PCM_TRIGGER_STOP:
-		amdtp_stream_pcm_trigger(&dice->rx_stream, NULL);
+		amdtp_stream_pcm_trigger(stream, NULL);
 		break;
 	default:
 		return -EINVAL;
@@ -258,14 +264,16 @@ static int playback_trigger(struct snd_pcm_substream *substream, int cmd)
 static snd_pcm_uframes_t capture_pointer(struct snd_pcm_substream *substream)
 {
 	struct snd_dice *dice = substream->private_data;
+	struct amdtp_stream *stream = &dice->tx_stream[0];
 
-	return amdtp_stream_pcm_pointer(&dice->tx_stream);
+	return amdtp_stream_pcm_pointer(stream);
 }
 static snd_pcm_uframes_t playback_pointer(struct snd_pcm_substream *substream)
 {
 	struct snd_dice *dice = substream->private_data;
+	struct amdtp_stream *stream = &dice->rx_stream[0];
 
-	return amdtp_stream_pcm_pointer(&dice->rx_stream);
+	return amdtp_stream_pcm_pointer(stream);
 }
 
 int snd_dice_create_pcm(struct snd_dice *dice)
diff --git a/sound/firewire/dice/dice-stream.c b/sound/firewire/dice/dice-stream.c
index df035b1..15d581d 100644
--- a/sound/firewire/dice/dice-stream.c
+++ b/sound/firewire/dice/dice-stream.c
@@ -72,7 +72,7 @@ static void release_resources(struct snd_dice *dice,
 
 	/* Reset channel number */
 	channel = cpu_to_be32((u32)-1);
-	if (resources == &dice->tx_resources)
+	if (resources == &dice->tx_resources[0])
 		snd_dice_transaction_write_tx(dice, TX_ISOCHRONOUS,
 					      &channel, sizeof(channel));
 	else
@@ -96,7 +96,7 @@ static int keep_resources(struct snd_dice *dice,
 
 	/* Set channel number */
 	channel = cpu_to_be32(resources->channel);
-	if (resources == &dice->tx_resources)
+	if (resources == &dice->tx_resources[0])
 		err = snd_dice_transaction_write_tx(dice, TX_ISOCHRONOUS,
 						    &channel, sizeof(channel));
 	else
@@ -113,10 +113,10 @@ static void stop_stream(struct snd_dice *dice, struct amdtp_stream *stream)
 	amdtp_stream_pcm_abort(stream);
 	amdtp_stream_stop(stream);
 
-	if (stream == &dice->tx_stream)
-		release_resources(dice, &dice->tx_resources);
+	if (stream == &dice->tx_stream[0])
+		release_resources(dice, &dice->tx_resources[0]);
 	else
-		release_resources(dice, &dice->rx_resources);
+		release_resources(dice, &dice->rx_resources[0]);
 }
 
 static int start_stream(struct snd_dice *dice, struct amdtp_stream *stream,
@@ -128,12 +128,12 @@ static int start_stream(struct snd_dice *dice, struct amdtp_stream *stream,
 	bool double_pcm_frames;
 	int err;
 
-	if (stream == &dice->tx_stream) {
-		resources = &dice->tx_resources;
+	if (stream == &dice->tx_stream[0]) {
+		resources = &dice->tx_resources[0];
 		err = snd_dice_transaction_read_tx(dice, TX_NUMBER_AUDIO,
 						   reg, sizeof(reg));
 	} else {
-		resources = &dice->rx_resources;
+		resources = &dice->rx_resources[0];
 		err = snd_dice_transaction_read_rx(dice, RX_NUMBER_AUDIO,
 						   reg, sizeof(reg));
 	}
@@ -200,8 +200,8 @@ int snd_dice_stream_start_duplex(struct snd_dice *dice, unsigned int rate)
 	if (dice->substreams_counter == 0)
 		goto end;
 
-	master = &dice->rx_stream;
-	slave  = &dice->tx_stream;
+	master = &dice->rx_stream[0];
+	slave  = &dice->tx_stream[0];
 
 	/* Some packet queueing errors. */
 	if (amdtp_streaming_error(master) || amdtp_streaming_error(slave))
@@ -275,8 +275,8 @@ void snd_dice_stream_stop_duplex(struct snd_dice *dice)
 
 	snd_dice_transaction_clear_enable(dice);
 
-	stop_stream(dice, &dice->tx_stream);
-	stop_stream(dice, &dice->rx_stream);
+	stop_stream(dice, &dice->tx_stream[0]);
+	stop_stream(dice, &dice->rx_stream[0]);
 }
 
 static int init_stream(struct snd_dice *dice, struct amdtp_stream *stream)
@@ -285,11 +285,11 @@ static int init_stream(struct snd_dice *dice, struct amdtp_stream *stream)
 	struct fw_iso_resources *resources;
 	enum amdtp_stream_direction dir;
 
-	if (stream == &dice->tx_stream) {
-		resources = &dice->tx_resources;
+	if (stream == &dice->tx_stream[0]) {
+		resources = &dice->tx_resources[0];
 		dir = AMDTP_IN_STREAM;
 	} else {
-		resources = &dice->rx_resources;
+		resources = &dice->rx_resources[0];
 		dir = AMDTP_OUT_STREAM;
 	}
 
@@ -315,10 +315,10 @@ static void destroy_stream(struct snd_dice *dice, struct amdtp_stream *stream)
 {
 	struct fw_iso_resources *resources;
 
-	if (stream == &dice->tx_stream)
-		resources = &dice->tx_resources;
+	if (stream == &dice->tx_stream[0])
+		resources = &dice->tx_resources[0];
 	else
-		resources = &dice->rx_resources;
+		resources = &dice->rx_resources[0];
 
 	amdtp_stream_destroy(stream);
 	fw_iso_resources_destroy(resources);
@@ -330,13 +330,13 @@ int snd_dice_stream_init_duplex(struct snd_dice *dice)
 
 	dice->substreams_counter = 0;
 
-	err = init_stream(dice, &dice->tx_stream);
+	err = init_stream(dice, &dice->tx_stream[0]);
 	if (err < 0)
 		goto end;
 
-	err = init_stream(dice, &dice->rx_stream);
+	err = init_stream(dice, &dice->rx_stream[0]);
 	if (err < 0)
-		destroy_stream(dice, &dice->tx_stream);
+		destroy_stream(dice, &dice->tx_stream[0]);
 end:
 	return err;
 }
@@ -345,8 +345,8 @@ void snd_dice_stream_destroy_duplex(struct snd_dice *dice)
 {
 	snd_dice_transaction_clear_enable(dice);
 
-	destroy_stream(dice, &dice->tx_stream);
-	destroy_stream(dice, &dice->rx_stream);
+	destroy_stream(dice, &dice->tx_stream[0]);
+	destroy_stream(dice, &dice->rx_stream[0]);
 
 	dice->substreams_counter = 0;
 }
@@ -363,11 +363,11 @@ void snd_dice_stream_update_duplex(struct snd_dice *dice)
 	 */
 	dice->global_enabled = false;
 
-	stop_stream(dice, &dice->rx_stream);
-	stop_stream(dice, &dice->tx_stream);
+	stop_stream(dice, &dice->rx_stream[0]);
+	stop_stream(dice, &dice->tx_stream[0]);
 
-	fw_iso_resources_update(&dice->rx_resources);
-	fw_iso_resources_update(&dice->tx_resources);
+	fw_iso_resources_update(&dice->rx_resources[0]);
+	fw_iso_resources_update(&dice->tx_resources[0]);
 }
 
 static void dice_lock_changed(struct snd_dice *dice)
diff --git a/sound/firewire/dice/dice.h b/sound/firewire/dice/dice.h
index 423cdba..8fba87d 100644
--- a/sound/firewire/dice/dice.h
+++ b/sound/firewire/dice/dice.h
@@ -39,6 +39,29 @@
 #include "../lib.h"
 #include "dice-interface.h"
 
+/*
+ * This module support maximum 2 pairs of tx/rx isochronous streams for
+ * our convinience.
+ *
+ * In documents for ASICs called with a name of 'DICE':
+ *  - ASIC for DICE II:
+ *   - Maximum 2 tx and 4 rx are supported.
+ *   - A packet supports maximum 16 data channels.
+ *  - TCD2210/2210-E (so-called 'Dice Mini'):
+ *   - Maximum 2 tx and 2 rx are supported.
+ *   - A packet supports maximum 16 data channels.
+ *  - TCD2220/2220-E (so-called 'Dice Jr.')
+ *   - 2 tx and 2 rx are supported.
+ *   - A packet supports maximum 16 data channels.
+ *  - TCD3070-CH (so-called 'Dice III')
+ *   - Maximum 2 tx and 2 rx are supported.
+ *   - A packet supports maximum 32 data channels.
+ *
+ * For the above, MIDI conformant data channel is just on the first isochronous
+ * stream.
+ */
+#define MAX_STREAMS	2
+
 struct snd_dice {
 	struct snd_card *card;
 	struct fw_unit *unit;
@@ -67,10 +90,10 @@ struct snd_dice {
 	wait_queue_head_t hwdep_wait;
 
 	/* For streaming */
-	struct fw_iso_resources tx_resources;
-	struct fw_iso_resources rx_resources;
-	struct amdtp_stream tx_stream;
-	struct amdtp_stream rx_stream;
+	struct fw_iso_resources tx_resources[MAX_STREAMS];
+	struct fw_iso_resources rx_resources[MAX_STREAMS];
+	struct amdtp_stream tx_stream[MAX_STREAMS];
+	struct amdtp_stream rx_stream[MAX_STREAMS];
 	bool global_enabled;
 	struct completion clock_accepted;
 	unsigned int substreams_counter;
-- 
2.7.0

[PATCH 2/4] ALSA: dice: handle whole available isochronous streams

[Takashi Sakamoto]

This commit enables ALSA dice driver to handle whole available streams.

In Dice, certain registers represent the number of available streams at
current sampling transfer frequency for both directions. The parameters
of each stream are represented in a block of register. This block is
aligned sequentially. These streams start simultaneously by writing
enable bit to a register.

This commit operates these registers when starting/stopping streams. I
note that this driver fails to handle dice units in which several streams
are available while IEEE 1394 host controllers support fewer isochronous
contexts.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/dice/dice-stream.c | 400 ++++++++++++++++++++++++--------------
 1 file changed, 257 insertions(+), 143 deletions(-)

diff --git a/sound/firewire/dice/dice-stream.c b/sound/firewire/dice/dice-stream.c
index 15d581d..e7658af 100644
--- a/sound/firewire/dice/dice-stream.c
+++ b/sound/firewire/dice/dice-stream.c
@@ -65,85 +65,85 @@ static int ensure_phase_lock(struct snd_dice *dice)
 	return 0;
 }
 
-static void release_resources(struct snd_dice *dice,
-			      struct fw_iso_resources *resources)
+static int get_register_params(struct snd_dice *dice, unsigned int params[4])
 {
-	__be32 channel;
-
-	/* Reset channel number */
-	channel = cpu_to_be32((u32)-1);
-	if (resources == &dice->tx_resources[0])
-		snd_dice_transaction_write_tx(dice, TX_ISOCHRONOUS,
-					      &channel, sizeof(channel));
-	else
-		snd_dice_transaction_write_rx(dice, RX_ISOCHRONOUS,
-					      &channel, sizeof(channel));
-
-	fw_iso_resources_free(resources);
-}
-
-static int keep_resources(struct snd_dice *dice,
-			  struct fw_iso_resources *resources,
-			  unsigned int max_payload_bytes)
-{
-	__be32 channel;
+	__be32 reg[2];
 	int err;
 
-	err = fw_iso_resources_allocate(resources, max_payload_bytes,
-				fw_parent_device(dice->unit)->max_speed);
+	err = snd_dice_transaction_read_tx(dice, TX_NUMBER, reg, sizeof(reg));
 	if (err < 0)
-		goto end;
+		return err;
+	params[0] = min_t(unsigned int, be32_to_cpu(reg[0]), MAX_STREAMS);
+	params[1] = be32_to_cpu(reg[1]) * 4;
 
-	/* Set channel number */
-	channel = cpu_to_be32(resources->channel);
-	if (resources == &dice->tx_resources[0])
-		err = snd_dice_transaction_write_tx(dice, TX_ISOCHRONOUS,
-						    &channel, sizeof(channel));
-	else
-		err = snd_dice_transaction_write_rx(dice, RX_ISOCHRONOUS,
-						    &channel, sizeof(channel));
+	err = snd_dice_transaction_read_rx(dice, RX_NUMBER, reg, sizeof(reg));
 	if (err < 0)
-		release_resources(dice, resources);
-end:
-	return err;
+		return err;
+	params[2] = min_t(unsigned int, be32_to_cpu(reg[0]), MAX_STREAMS);
+	params[3] = be32_to_cpu(reg[1]) * 4;
+
+	return 0;
 }
 
-static void stop_stream(struct snd_dice *dice, struct amdtp_stream *stream)
+static void release_resources(struct snd_dice *dice)
 {
-	amdtp_stream_pcm_abort(stream);
-	amdtp_stream_stop(stream);
+	unsigned int i;
+
+	for (i = 0; i < MAX_STREAMS; i++) {
+		if (amdtp_stream_running(&dice->tx_stream[i])) {
+			amdtp_stream_pcm_abort(&dice->tx_stream[i]);
+			amdtp_stream_stop(&dice->tx_stream[i]);
+		}
+		if (amdtp_stream_running(&dice->rx_stream[i])) {
+			amdtp_stream_pcm_abort(&dice->rx_stream[i]);
+			amdtp_stream_stop(&dice->rx_stream[i]);
+		}
 
-	if (stream == &dice->tx_stream[0])
-		release_resources(dice, &dice->tx_resources[0]);
-	else
-		release_resources(dice, &dice->rx_resources[0]);
+		fw_iso_resources_free(&dice->tx_resources[i]);
+		fw_iso_resources_free(&dice->rx_resources[i]);
+	}
 }
 
-static int start_stream(struct snd_dice *dice, struct amdtp_stream *stream,
-			unsigned int rate)
+static void stop_streams(struct snd_dice *dice, enum amdtp_stream_direction dir,
+			 unsigned int count, unsigned int size)
 {
+	__be32 reg;
+	unsigned int i;
+
+	for (i = 0; i < count; i++) {
+		/* 0x0008 is TX_ISOCHRONOUS/RX_ISOCHRONOUS. */
+		reg = cpu_to_be32((u32)-1);
+		if (dir == AMDTP_IN_STREAM) {
+			snd_dice_transaction_write_tx(dice,
+						size * i + TX_ISOCHRONOUS,
+						&reg, sizeof(reg));
+		} else {
+			snd_dice_transaction_write_rx(dice,
+						size * i + RX_ISOCHRONOUS,
+						&reg, sizeof(reg));
+		}
+	}
+}
+
+static int keep_resources(struct snd_dice *dice,
+			  enum amdtp_stream_direction dir, unsigned int index,
+			  unsigned int rate, unsigned int pcm_chs,
+			  unsigned int midi_ports)
+{
+	struct amdtp_stream *stream;
 	struct fw_iso_resources *resources;
-	__be32 reg[2];
-	unsigned int i, pcm_chs, midi_ports;
 	bool double_pcm_frames;
+	unsigned int i;
 	int err;
 
-	if (stream == &dice->tx_stream[0]) {
-		resources = &dice->tx_resources[0];
-		err = snd_dice_transaction_read_tx(dice, TX_NUMBER_AUDIO,
-						   reg, sizeof(reg));
+	if (dir == AMDTP_IN_STREAM) {
+		stream = &dice->tx_stream[index];
+		resources = &dice->tx_resources[index];
 	} else {
-		resources = &dice->rx_resources[0];
-		err = snd_dice_transaction_read_rx(dice, RX_NUMBER_AUDIO,
-						   reg, sizeof(reg));
+		stream = &dice->rx_stream[index];
+		resources = &dice->rx_resources[index];
 	}
 
-	if (err < 0)
-		goto end;
-
-	pcm_chs = be32_to_cpu(reg[0]);
-	midi_ports = be32_to_cpu(reg[1]);
-
 	/*
 	 * At 176.4/192.0 kHz, Dice has a quirk to transfer two PCM frames in
 	 * one data block of AMDTP packet. Thus sampling transfer frequency is
@@ -163,7 +163,7 @@ static int start_stream(struct snd_dice *dice, struct amdtp_stream *stream,
 	err = amdtp_am824_set_parameters(stream, rate, pcm_chs, midi_ports,
 					 double_pcm_frames);
 	if (err < 0)
-		goto end;
+		return err;
 
 	if (double_pcm_frames) {
 		pcm_chs /= 2;
@@ -175,122 +175,209 @@ static int start_stream(struct snd_dice *dice, struct amdtp_stream *stream,
 		}
 	}
 
-	err = keep_resources(dice, resources,
-			     amdtp_stream_get_max_payload(stream));
-	if (err < 0) {
-		dev_err(&dice->unit->device,
-			"fail to keep isochronous resources\n");
-		goto end;
+	return fw_iso_resources_allocate(resources,
+				amdtp_stream_get_max_payload(stream),
+				fw_parent_device(dice->unit)->max_speed);
+}
+
+static int start_streams(struct snd_dice *dice, enum amdtp_stream_direction dir,
+			 unsigned int rate, unsigned int count,
+			 unsigned int size)
+{
+	__be32 reg[2];
+	unsigned int i, pcm_chs, midi_ports;
+	struct amdtp_stream *streams;
+	struct fw_iso_resources *resources;
+	int err = 0;
+
+	if (dir == AMDTP_IN_STREAM) {
+		streams = dice->tx_stream;
+		resources = dice->tx_resources;
+	} else {
+		streams = dice->rx_stream;
+		resources = dice->rx_resources;
+	}
+
+	for (i = 0; i < count; i++) {
+		if (dir == AMDTP_IN_STREAM) {
+			err = snd_dice_transaction_read_tx(dice,
+						size * i + TX_NUMBER_AUDIO,
+						reg, sizeof(reg));
+		} else {
+			err = snd_dice_transaction_read_rx(dice,
+						size * i + RX_NUMBER_AUDIO,
+						reg, sizeof(reg));
+		}
+		if (err < 0)
+			return err;
+		pcm_chs = be32_to_cpu(reg[0]);
+		midi_ports = be32_to_cpu(reg[1]);
+
+		err = keep_resources(dice, dir, i, rate, pcm_chs, midi_ports);
+		if (err < 0)
+			return err;
+
+		/* 0x0008 is TX_ISOCHRONOUS/RX_ISOCHRONOUS. */
+		reg[0] = cpu_to_be32(resources[i].channel);
+		if (dir == AMDTP_IN_STREAM) {
+			err = snd_dice_transaction_write_tx(dice,
+						size * i + TX_ISOCHRONOUS,
+						reg, sizeof(reg[0]));
+		} else {
+			err = snd_dice_transaction_write_rx(dice,
+						size * i + RX_ISOCHRONOUS,
+						reg, sizeof(reg[0]));
+		}
+		if (err < 0)
+			return err;
+
+		err = amdtp_stream_start(&streams[i], resources[i].channel,
+				fw_parent_device(dice->unit)->max_speed);
+		if (err < 0)
+			return err;
 	}
 
-	err = amdtp_stream_start(stream, resources->channel,
-				 fw_parent_device(dice->unit)->max_speed);
-	if (err < 0)
-		release_resources(dice, resources);
-end:
 	return err;
 }
 
+/*
+ * MEMO: After this function, there're two states of streams:
+ *  - None streams are running.
+ *  - All streams are running.
+ */
 int snd_dice_stream_start_duplex(struct snd_dice *dice, unsigned int rate)
 {
-	struct amdtp_stream *master, *slave;
 	unsigned int curr_rate;
-	int err = 0;
+	unsigned int i;
+	unsigned int reg_params[4];
+	bool need_to_start;
+	int err;
 
 	if (dice->substreams_counter == 0)
-		goto end;
-
-	master = &dice->rx_stream[0];
-	slave  = &dice->tx_stream[0];
+		return -EIO;
 
-	/* Some packet queueing errors. */
-	if (amdtp_streaming_error(master) || amdtp_streaming_error(slave))
-		stop_stream(dice, master);
+	err = get_register_params(dice, reg_params);
+	if (err < 0)
+		return err;
 
-	/* Stop stream if rate is different. */
 	err = snd_dice_transaction_get_rate(dice, &curr_rate);
 	if (err < 0) {
 		dev_err(&dice->unit->device,
 			"fail to get sampling rate\n");
-		goto end;
+		return err;
 	}
 	if (rate == 0)
 		rate = curr_rate;
-	if (rate != curr_rate) {
-		err = -EINVAL;
-		goto end;
+	if (rate != curr_rate)
+		return -EINVAL;
+
+	/* Judge to need to restart streams. */
+	for (i = 0; i < MAX_STREAMS; i++) {
+		if (i < reg_params[0]) {
+			if (amdtp_streaming_error(&dice->tx_stream[i]) ||
+			    !amdtp_stream_running(&dice->tx_stream[i]))
+				break;
+		}
+		if (i < reg_params[2]) {
+			if (amdtp_streaming_error(&dice->rx_stream[i]) ||
+			    !amdtp_stream_running(&dice->rx_stream[i]))
+				break;
+		}
 	}
+	need_to_start = (i < MAX_STREAMS);
 
-	if (!amdtp_stream_running(master)) {
-		stop_stream(dice, slave);
+	if (need_to_start) {
+		/* Stop transmission. */
 		snd_dice_transaction_clear_enable(dice);
+		stop_streams(dice, AMDTP_IN_STREAM, reg_params[0],
+			     reg_params[1]);
+		stop_streams(dice, AMDTP_OUT_STREAM, reg_params[2],
+			     reg_params[3]);
+		release_resources(dice);
 
 		err = ensure_phase_lock(dice);
 		if (err < 0) {
 			dev_err(&dice->unit->device,
 				"fail to ensure phase lock\n");
-			goto end;
+			return err;
 		}
 
 		/* Start both streams. */
-		err = start_stream(dice, master, rate);
-		if (err < 0) {
-			dev_err(&dice->unit->device,
-				"fail to start AMDTP master stream\n");
-			goto end;
-		}
-		err = start_stream(dice, slave, rate);
-		if (err < 0) {
-			dev_err(&dice->unit->device,
-				"fail to start AMDTP slave stream\n");
-			stop_stream(dice, master);
-			goto end;
-		}
+		err = start_streams(dice, AMDTP_IN_STREAM, rate, reg_params[0],
+				    reg_params[1]);
+		if (err < 0)
+			goto error;
+		err = start_streams(dice, AMDTP_OUT_STREAM, rate, reg_params[2],
+				    reg_params[3]);
+		if (err < 0)
+			goto error;
+
 		err = snd_dice_transaction_set_enable(dice);
 		if (err < 0) {
 			dev_err(&dice->unit->device,
 				"fail to enable interface\n");
-			stop_stream(dice, master);
-			stop_stream(dice, slave);
-			goto end;
+			goto error;
 		}
 
-		/* Wait first callbacks */
-		if (!amdtp_stream_wait_callback(master, CALLBACK_TIMEOUT) ||
-		    !amdtp_stream_wait_callback(slave, CALLBACK_TIMEOUT)) {
-			snd_dice_transaction_clear_enable(dice);
-			stop_stream(dice, master);
-			stop_stream(dice, slave);
-			err = -ETIMEDOUT;
+		for (i = 0; i < MAX_STREAMS; i++) {
+			if ((i < reg_params[0] &&
+			    !amdtp_stream_wait_callback(&dice->tx_stream[i],
+							CALLBACK_TIMEOUT)) ||
+			    (i < reg_params[2] &&
+			     !amdtp_stream_wait_callback(&dice->rx_stream[i],
+							 CALLBACK_TIMEOUT))) {
+				err = -ETIMEDOUT;
+				goto error;
+			}
 		}
 	}
-end:
+
+	return err;
+error:
+	snd_dice_transaction_clear_enable(dice);
+	stop_streams(dice, AMDTP_IN_STREAM, reg_params[0], reg_params[1]);
+	stop_streams(dice, AMDTP_OUT_STREAM, reg_params[2], reg_params[3]);
+	release_resources(dice);
 	return err;
 }
 
+/*
+ * MEMO: After this function, there're two states of streams:
+ *  - None streams are running.
+ *  - All streams are running.
+ */
 void snd_dice_stream_stop_duplex(struct snd_dice *dice)
 {
+	unsigned int reg_params[4];
+
 	if (dice->substreams_counter > 0)
 		return;
 
 	snd_dice_transaction_clear_enable(dice);
 
-	stop_stream(dice, &dice->tx_stream[0]);
-	stop_stream(dice, &dice->rx_stream[0]);
+	if (get_register_params(dice, reg_params) == 0) {
+		stop_streams(dice, AMDTP_IN_STREAM, reg_params[0],
+			     reg_params[1]);
+		stop_streams(dice, AMDTP_OUT_STREAM, reg_params[2],
+			     reg_params[3]);
+	}
+
+	release_resources(dice);
 }
 
-static int init_stream(struct snd_dice *dice, struct amdtp_stream *stream)
+static int init_stream(struct snd_dice *dice, enum amdtp_stream_direction dir,
+		       unsigned int index)
 {
-	int err;
+	struct amdtp_stream *stream;
 	struct fw_iso_resources *resources;
-	enum amdtp_stream_direction dir;
+	int err;
 
-	if (stream == &dice->tx_stream[0]) {
-		resources = &dice->tx_resources[0];
-		dir = AMDTP_IN_STREAM;
+	if (dir == AMDTP_IN_STREAM) {
+		stream = &dice->tx_stream[index];
+		resources = &dice->tx_resources[index];
 	} else {
-		resources = &dice->rx_resources[0];
-		dir = AMDTP_OUT_STREAM;
+		stream = &dice->rx_stream[index];
+		resources = &dice->rx_resources[index];
 	}
 
 	err = fw_iso_resources_init(resources, dice->unit);
@@ -311,14 +398,20 @@ end:
  * This function should be called before starting streams or after stopping
  * streams.
  */
-static void destroy_stream(struct snd_dice *dice, struct amdtp_stream *stream)
+static void destroy_stream(struct snd_dice *dice,
+			   enum amdtp_stream_direction dir,
+			   unsigned int index)
 {
+	struct amdtp_stream *stream;
 	struct fw_iso_resources *resources;
 
-	if (stream == &dice->tx_stream[0])
-		resources = &dice->tx_resources[0];
-	else
-		resources = &dice->rx_resources[0];
+	if (dir == AMDTP_IN_STREAM) {
+		stream = &dice->tx_stream[index];
+		resources = &dice->tx_resources[index];
+	} else {
+		stream = &dice->rx_stream[index];
+		resources = &dice->rx_resources[index];
+	}
 
 	amdtp_stream_destroy(stream);
 	fw_iso_resources_destroy(resources);
@@ -326,33 +419,53 @@ static void destroy_stream(struct snd_dice *dice, struct amdtp_stream *stream)
 
 int snd_dice_stream_init_duplex(struct snd_dice *dice)
 {
-	int err;
+	int i, err;
 
-	dice->substreams_counter = 0;
-
-	err = init_stream(dice, &dice->tx_stream[0]);
-	if (err < 0)
-		goto end;
+	for (i = 0; i < MAX_STREAMS; i++) {
+		err = init_stream(dice, AMDTP_IN_STREAM, i);
+		if (err < 0) {
+			for (; i >= 0; i--)
+				destroy_stream(dice, AMDTP_OUT_STREAM, i);
+			goto end;
+		}
+	}
 
-	err = init_stream(dice, &dice->rx_stream[0]);
-	if (err < 0)
-		destroy_stream(dice, &dice->tx_stream[0]);
+	for (i = 0; i < MAX_STREAMS; i++) {
+		err = init_stream(dice, AMDTP_OUT_STREAM, i);
+		if (err < 0) {
+			for (; i >= 0; i--)
+				destroy_stream(dice, AMDTP_OUT_STREAM, i);
+			for (i = 0; i < MAX_STREAMS; i++)
+				destroy_stream(dice, AMDTP_IN_STREAM, i);
+			break;
+		}
+	}
 end:
 	return err;
 }
 
 void snd_dice_stream_destroy_duplex(struct snd_dice *dice)
 {
+	unsigned int reg_params[4];
+
 	snd_dice_transaction_clear_enable(dice);
 
-	destroy_stream(dice, &dice->tx_stream[0]);
-	destroy_stream(dice, &dice->rx_stream[0]);
+	if (get_register_params(dice, reg_params) == 0) {
+		stop_streams(dice, AMDTP_IN_STREAM, reg_params[0],
+			     reg_params[1]);
+		stop_streams(dice, AMDTP_OUT_STREAM, reg_params[2],
+			     reg_params[3]);
+	}
+
+	release_resources(dice);
 
 	dice->substreams_counter = 0;
 }
 
 void snd_dice_stream_update_duplex(struct snd_dice *dice)
 {
+	unsigned int reg_params[4];
+
 	/*
 	 * On a bus reset, the DICE firmware disables streaming and then goes
 	 * off contemplating its own navel for hundreds of milliseconds before
@@ -363,11 +476,12 @@ void snd_dice_stream_update_duplex(struct snd_dice *dice)
 	 */
 	dice->global_enabled = false;
 
-	stop_stream(dice, &dice->rx_stream[0]);
-	stop_stream(dice, &dice->tx_stream[0]);
-
-	fw_iso_resources_update(&dice->rx_resources[0]);
-	fw_iso_resources_update(&dice->tx_resources[0]);
+	if (get_register_params(dice, reg_params) == 0) {
+		stop_streams(dice, AMDTP_IN_STREAM, reg_params[0],
+			     reg_params[1]);
+		stop_streams(dice, AMDTP_OUT_STREAM, reg_params[2],
+			     reg_params[3]);
+	}
 }
 
 static void dice_lock_changed(struct snd_dice *dice)
-- 
2.7.0

Re: [PATCH 2/4] ALSA: dice: handle whole available isochronous streams

[Stefan Richter]

On Mar 05 Takashi Sakamoto wrote:
> This commit enables ALSA dice driver to handle whole available streams.
> 
> In Dice, certain registers represent the number of available streams at
> current sampling transfer frequency for both directions. The parameters
> of each stream are represented in a block of register. This block is
> aligned sequentially. These streams start simultaneously by writing
> enable bit to a register.
> 
> This commit operates these registers when starting/stopping streams. I
> note that this driver fails to handle dice units in which several streams
> are available while IEEE 1394 host controllers support fewer isochronous
> contexts.

Regarding support by host controllers:

OHCI-1394 mandates that host controllers implement at least 4 isochronous
RX DMAs and at least 4 isochronous TX DMAs.  None of the OHCI controllers
that ever came to market violate this requirement as far as I know.

So if you have just a single audio device on the bus, or even two, the
controller won't run out of DMA contexts.

Regarding non-audio FireWire applications:  Most IEEE 1394 video cameras
require one RX stream per camera; some industrial high-bandwidth cameras
take two.  Some DV camcorders offer bidirectional DV transfer over 1394,
hence use 1 RX and/or 1 TX stream depending on the application software on
the host.  The IP-over-1394 protocol, implemented in Linux by the
firewire-net driver, uses 1 isochronous RX DMA context for reception of
asynchronous broadcast transmissions.  The SBP-2 protocol, mainly used as
SCSI-over-1394 encapsulation, does not use isochronous streams.  The
current SBP-3 protocol revision specifies optional isochronous I/O but I
am not aware of any implementation of this protocol feature.

On the other hand, while all OHCIs offer at least 4+4 isoch RX+TX DMAs,
practical experience with the FFADO userspace drivers indicate that most
OHCIs quickly become unreliable when more contexts are being used
simultaneously.  Particularly, the FFADO drivers have a hard time to
maintain synchronicity of the streams, or are flat out unable to establish
synchronization.  Though from what I understand, the ALSA firewire drivers
do this differently, hence may not be affected in the same way as FFADO.
In my and others experience, only LSI controllers (formerly Agere
controllers) and Texas Instruments controllers do not suffer from these
issues when multiple RX/TX streams are active in FFADO.
-- 
Stefan Richter
-======----- --== --=-=
http://arcgraph.de/sr/

[PATCH 3/4] ALSA: dice: handle several PCM substreams when any isochronous streams are available

[Takashi Sakamoto]

In former commits, ALSA dice driver can handle available isochronous
streams. This commit adds support for several PCM substreams on the
streams.

The additional PCM substreams are available via subdevice of ALSA PCM
character devices. For example, two PCM substreams are available on
the streams, the second PCM substream is handled via second subdevice
of the character device. In configuration space of alsa-lib, it's
represented with 'hw:0,0,1'

The PCM substreams are constraint to parameters of the corresponding
streams. If for some reasons, the PCM substreams are unavailable,
open(2) to ALSA PCM character device returns error and reports ENXIO.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/dice/dice-pcm.c | 82 +++++++++++++++++++++++++-----------------
 1 file changed, 49 insertions(+), 33 deletions(-)

diff --git a/sound/firewire/dice/dice-pcm.c b/sound/firewire/dice/dice-pcm.c
index e252949..3762bd3 100644
--- a/sound/firewire/dice/dice-pcm.c
+++ b/sound/firewire/dice/dice-pcm.c
@@ -11,28 +11,34 @@
 
 static int limit_channels_and_rates(struct snd_dice *dice,
 				    struct snd_pcm_runtime *runtime,
-				    struct amdtp_stream *stream)
+				    enum amdtp_stream_direction dir,
+				    unsigned int index, unsigned int size)
 {
 	struct snd_pcm_hardware *hw = &runtime->hw;
+	struct amdtp_stream *stream;
 	unsigned int rate;
-	__be32 reg[2];
+	__be32 reg;
 	int err;
 
 	/*
 	 * Retrieve current Multi Bit Linear Audio data channel and limit to
 	 * it.
 	 */
-	if (stream == &dice->tx_stream[0]) {
-		err = snd_dice_transaction_read_tx(dice, TX_NUMBER_AUDIO,
-						   reg, sizeof(reg));
+	if (dir == AMDTP_IN_STREAM) {
+		stream = &dice->tx_stream[index];
+		err = snd_dice_transaction_read_tx(dice,
+				size * index + TX_NUMBER_AUDIO,
+				&reg, sizeof(reg));
 	} else {
-		err = snd_dice_transaction_read_rx(dice, RX_NUMBER_AUDIO,
-						   reg, sizeof(reg));
+		stream = &dice->rx_stream[index];
+		err = snd_dice_transaction_read_rx(dice,
+				size * index + RX_NUMBER_AUDIO,
+				&reg, sizeof(reg));
 	}
 	if (err < 0)
 		return err;
 
-	hw->channels_min = hw->channels_max = be32_to_cpu(reg[0]);
+	hw->channels_min = hw->channels_max = be32_to_cpu(reg);
 
 	/* Retrieve current sampling transfer frequency and limit to it. */
 	err = snd_dice_transaction_get_rate(dice, &rate);
@@ -62,7 +68,10 @@ static int init_hw_info(struct snd_dice *dice,
 {
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	struct snd_pcm_hardware *hw = &runtime->hw;
+	enum amdtp_stream_direction dir;
 	struct amdtp_stream *stream;
+	__be32 reg[2];
+	unsigned int count, size;
 	int err;
 
 	hw->info = SNDRV_PCM_INFO_MMAP |
@@ -74,13 +83,28 @@ static int init_hw_info(struct snd_dice *dice,
 
 	if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) {
 		hw->formats = AM824_IN_PCM_FORMAT_BITS;
-		stream = &dice->tx_stream[0];
+		dir = AMDTP_IN_STREAM;
+		stream = &dice->tx_stream[substream->number];
+		err = snd_dice_transaction_read_tx(dice, TX_NUMBER, reg,
+						   sizeof(reg));
 	} else {
 		hw->formats = AM824_OUT_PCM_FORMAT_BITS;
-		stream = &dice->rx_stream[0];
+		dir = AMDTP_OUT_STREAM;
+		stream = &dice->rx_stream[substream->number];
+		err = snd_dice_transaction_read_rx(dice, RX_NUMBER, reg,
+						   sizeof(reg));
 	}
 
-	err = limit_channels_and_rates(dice, runtime, stream);
+	if (err < 0)
+		return err;
+
+	count = min_t(unsigned int, be32_to_cpu(reg[0]), MAX_STREAMS);
+	if (substream->number >= count)
+		return -ENXIO;
+
+	size = be32_to_cpu(reg[1]) * 4;
+	err = limit_channels_and_rates(dice, substream->runtime, dir,
+				       substream->number, size);
 	if (err < 0)
 		return err;
 	limit_period_and_buffer(hw);
@@ -122,7 +146,7 @@ static int capture_hw_params(struct snd_pcm_substream *substream,
 			     struct snd_pcm_hw_params *hw_params)
 {
 	struct snd_dice *dice = substream->private_data;
-	struct amdtp_stream *stream = &dice->tx_stream[0];
+	struct amdtp_stream *stream = &dice->tx_stream[substream->number];
 	int err;
 
 	err = snd_pcm_lib_alloc_vmalloc_buffer(substream,
@@ -144,7 +168,7 @@ static int playback_hw_params(struct snd_pcm_substream *substream,
 			      struct snd_pcm_hw_params *hw_params)
 {
 	struct snd_dice *dice = substream->private_data;
-	struct amdtp_stream *stream = &dice->rx_stream[0];
+	struct amdtp_stream *stream = &dice->rx_stream[substream->number];
 	int err;
 
 	err = snd_pcm_lib_alloc_vmalloc_buffer(substream,
@@ -198,7 +222,7 @@ static int playback_hw_free(struct snd_pcm_substream *substream)
 static int capture_prepare(struct snd_pcm_substream *substream)
 {
 	struct snd_dice *dice = substream->private_data;
-	struct amdtp_stream *stream = &dice->tx_stream[0];
+	struct amdtp_stream *stream = &dice->tx_stream[substream->number];
 	int err;
 
 	mutex_lock(&dice->mutex);
@@ -212,7 +236,7 @@ static int capture_prepare(struct snd_pcm_substream *substream)
 static int playback_prepare(struct snd_pcm_substream *substream)
 {
 	struct snd_dice *dice = substream->private_data;
-	struct amdtp_stream *stream = &dice->rx_stream[0];
+	struct amdtp_stream *stream = &dice->rx_stream[substream->number];
 	int err;
 
 	mutex_lock(&dice->mutex);
@@ -227,7 +251,7 @@ static int playback_prepare(struct snd_pcm_substream *substream)
 static int capture_trigger(struct snd_pcm_substream *substream, int cmd)
 {
 	struct snd_dice *dice = substream->private_data;
-	struct amdtp_stream *stream = &dice->tx_stream[0];
+	struct amdtp_stream *stream = &dice->tx_stream[substream->number];
 
 	switch (cmd) {
 	case SNDRV_PCM_TRIGGER_START:
@@ -245,7 +269,7 @@ static int capture_trigger(struct snd_pcm_substream *substream, int cmd)
 static int playback_trigger(struct snd_pcm_substream *substream, int cmd)
 {
 	struct snd_dice *dice = substream->private_data;
-	struct amdtp_stream *stream = &dice->rx_stream[0];
+	struct amdtp_stream *stream = &dice->rx_stream[substream->number];
 
 	switch (cmd) {
 	case SNDRV_PCM_TRIGGER_START:
@@ -264,14 +288,14 @@ static int playback_trigger(struct snd_pcm_substream *substream, int cmd)
 static snd_pcm_uframes_t capture_pointer(struct snd_pcm_substream *substream)
 {
 	struct snd_dice *dice = substream->private_data;
-	struct amdtp_stream *stream = &dice->tx_stream[0];
+	struct amdtp_stream *stream = &dice->tx_stream[substream->number];
 
 	return amdtp_stream_pcm_pointer(stream);
 }
 static snd_pcm_uframes_t playback_pointer(struct snd_pcm_substream *substream)
 {
 	struct snd_dice *dice = substream->private_data;
-	struct amdtp_stream *stream = &dice->rx_stream[0];
+	struct amdtp_stream *stream = &dice->rx_stream[substream->number];
 
 	return amdtp_stream_pcm_pointer(stream);
 }
@@ -307,25 +331,17 @@ int snd_dice_create_pcm(struct snd_dice *dice)
 	unsigned int capture, playback;
 	int err;
 
-	/*
-	 * Check whether PCM substreams are required.
-	 *
-	 * TODO: in the case that any PCM substreams are not avail at a certain
-	 * sampling transfer frequency?
-	 */
-	err = snd_dice_transaction_read_tx(dice, TX_NUMBER_AUDIO,
-					   &reg, sizeof(reg));
+	/* Check whether PCM substreams are required. */
+	capture = playback = 0;
+	err = snd_dice_transaction_read_tx(dice, TX_NUMBER, &reg, sizeof(reg));
 	if (err < 0)
 		return err;
-	if (be32_to_cpu(reg) > 0)
-		capture = 1;
+	capture = min_t(unsigned int, be32_to_cpu(reg), MAX_STREAMS);
 
-	err = snd_dice_transaction_read_rx(dice, RX_NUMBER_AUDIO,
-					   &reg, sizeof(reg));
+	err = snd_dice_transaction_read_rx(dice, RX_NUMBER, &reg, sizeof(reg));
 	if (err < 0)
 		return err;
-	if (be32_to_cpu(reg) > 0)
-		playback = 1;
+	playback = min_t(unsigned int, be32_to_cpu(reg), MAX_STREAMS);
 
 	err = snd_pcm_new(dice->card, "DICE", 0, playback, capture, &pcm);
 	if (err < 0)
-- 
2.7.0

[PATCH 4/4] ALSA: dice: force to add two pcm devices for listed models

[Takashi Sakamoto]

Some models reduce the number of available isochronous streams for higher
sampling transfer frequency. Such models bring an issue about how to add
PCM substreams. When at lower sampling transfer frequency, the
models reports whole available streams, thus this driver can add enough
number of PCM substreams at probing time. On the other hand, at higher
sampling transfer frequency, this driver can just add reduced number of
PCM substreams. After probed, even if the sampling transfer frequency is
changed to lower rate, fewer PCM substreams are actually available. This
is inconvenience.

For the reason, this commit adds a list so that this driver assume models
on the list to have two pairs of PCM substreams. This list keeps the name
of model in which the number of available streams differs depending on
sampling transfer frequency.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/dice/dice-pcm.c | 24 +++++++++++++++---------
 sound/firewire/dice/dice.c     | 41 +++++++++++++++++++++++++++++++++++++++++
 sound/firewire/dice/dice.h     |  2 ++
 3 files changed, 58 insertions(+), 9 deletions(-)

diff --git a/sound/firewire/dice/dice-pcm.c b/sound/firewire/dice/dice-pcm.c
index 3762bd3..532de99 100644
--- a/sound/firewire/dice/dice-pcm.c
+++ b/sound/firewire/dice/dice-pcm.c
@@ -332,16 +332,22 @@ int snd_dice_create_pcm(struct snd_dice *dice)
 	int err;
 
 	/* Check whether PCM substreams are required. */
-	capture = playback = 0;
-	err = snd_dice_transaction_read_tx(dice, TX_NUMBER, &reg, sizeof(reg));
-	if (err < 0)
-		return err;
-	capture = min_t(unsigned int, be32_to_cpu(reg), MAX_STREAMS);
+	if (dice->force_two_pcms) {
+		capture = playback = 2;
+	} else {
+		capture = playback = 0;
+		err = snd_dice_transaction_read_tx(dice, TX_NUMBER, &reg,
+						   sizeof(reg));
+		if (err < 0)
+			return err;
+		capture = min_t(unsigned int, be32_to_cpu(reg), MAX_STREAMS);
 
-	err = snd_dice_transaction_read_rx(dice, RX_NUMBER, &reg, sizeof(reg));
-	if (err < 0)
-		return err;
-	playback = min_t(unsigned int, be32_to_cpu(reg), MAX_STREAMS);
+		err = snd_dice_transaction_read_rx(dice, RX_NUMBER, &reg,
+						   sizeof(reg));
+		if (err < 0)
+			return err;
+		playback = min_t(unsigned int, be32_to_cpu(reg), MAX_STREAMS);
+	}
 
 	err = snd_pcm_new(dice->card, "DICE", 0, playback, capture, &pcm);
 	if (err < 0)
diff --git a/sound/firewire/dice/dice.c b/sound/firewire/dice/dice.c
index f7303a6..215b6cc 100644
--- a/sound/firewire/dice/dice.c
+++ b/sound/firewire/dice/dice.c
@@ -13,6 +13,8 @@ MODULE_LICENSE("GPL v2");
 
 #define OUI_WEISS		0x001c6a
 #define OUI_LOUD		0x000ff2
+#define OUI_FOCUSRITE		0x00130e
+#define OUI_TCELECTRONIC	0x001486
 
 #define DICE_CATEGORY_ID	0x04
 #define WEISS_CATEGORY_ID	0x00
@@ -20,6 +22,36 @@ MODULE_LICENSE("GPL v2");
 
 #define PROBE_DELAY_MS		(2 * MSEC_PER_SEC)
 
+/*
+ * Some models support several isochronous channels, while these streams are not
+ * always available. In this case, add the model name to this list.
+ */
+static bool force_two_pcm_support(struct fw_unit *unit)
+{
+	const char *const models[] = {
+		/* TC Electronic models. */
+		"StudioKonnekt48",
+		/* Focusrite models. */
+		"SAFFIRE PRO 40",
+		"LIQUID SAFFIRE 56",
+		"SAFFIRE PRO 40 1",
+	};
+	char model[32];
+	unsigned int i;
+	int err;
+
+	err = fw_csr_string(unit->directory, CSR_MODEL, model, sizeof(model));
+	if (err < 0)
+		return false;
+
+	for (i = 0; i < ARRAY_SIZE(models); i++) {
+		if (strcmp(models[i], model) == 0)
+			break;
+	}
+
+	return i < ARRAY_SIZE(models);
+}
+
 static int check_dice_category(struct fw_unit *unit)
 {
 	struct fw_device *device = fw_parent_device(unit);
@@ -44,6 +76,12 @@ static int check_dice_category(struct fw_unit *unit)
 			break;
 		}
 	}
+
+	if (vendor == OUI_FOCUSRITE || vendor == OUI_TCELECTRONIC) {
+		if (force_two_pcm_support(unit))
+			return 0;
+	}
+
 	if (vendor == OUI_WEISS)
 		category = WEISS_CATEGORY_ID;
 	else if (vendor == OUI_LOUD)
@@ -150,6 +188,9 @@ static void do_registration(struct work_struct *work)
 	if (err < 0)
 		return;
 
+	if (force_two_pcm_support(dice->unit))
+		dice->force_two_pcms = true;
+
 	err = snd_dice_transaction_init(dice);
 	if (err < 0)
 		goto error;
diff --git a/sound/firewire/dice/dice.h b/sound/firewire/dice/dice.h
index 8fba87d..e6c0785 100644
--- a/sound/firewire/dice/dice.h
+++ b/sound/firewire/dice/dice.h
@@ -97,6 +97,8 @@ struct snd_dice {
 	bool global_enabled;
 	struct completion clock_accepted;
 	unsigned int substreams_counter;
+
+	bool force_two_pcms;
 };
 
 enum snd_dice_addr_type {
-- 
2.7.0

Re: [PATCH 4/4] ALSA: dice: force to add two pcm devices for listed models

[Stefan Richter]

On Mar 05 Takashi Sakamoto wrote:
> +static bool force_two_pcm_support(struct fw_unit *unit)
> +{
> +	const char *const models[] = {
> +		/* TC Electronic models. */
> +		"StudioKonnekt48",
> +		/* Focusrite models. */
> +		"SAFFIRE PRO 40",
> +		"LIQUID SAFFIRE 56",
> +		"SAFFIRE PRO 40 1",
> +	};
> +	char model[32];
> +	unsigned int i;
> +	int err;
> +
> +	err = fw_csr_string(unit->directory, CSR_MODEL, model, sizeof(model));
> +	if (err < 0)
> +		return false;
> +
> +	for (i = 0; i < ARRAY_SIZE(models); i++) {
> +		if (strcmp(models[i], model) == 0)
> +			break;
> +	}
> +
> +	return i < ARRAY_SIZE(models);
> +}

The model name of my old Saffire PRO 40 is "SAFFIRE_PRO_40", and
according to e.g. Alban Bernard on ffado-user, the newer model is
"SAFFIRE_PRO_40_1".  Saffire 56's name is "LIQUID_SAFFIRE_56"
according to https://forum.ubuntuusers.de/topic/focusrite-liquid/.
-- 
Stefan Richter
-======----- --== --=-=
http://arcgraph.de/sr/

Re: [PATCH 4/4] ALSA: dice: force to add two pcm devices for listed models

[Takashi Sakamoto]

On Mar 6 2016 00:07, Stefan Richter wrote:
> On Mar 05 Takashi Sakamoto wrote:
>> +static bool force_two_pcm_support(struct fw_unit *unit)
>> +{
>> +	const char *const models[] = {
>> +		/* TC Electronic models. */
>> +		"StudioKonnekt48",
>> +		/* Focusrite models. */
>> +		"SAFFIRE PRO 40",
>> +		"LIQUID SAFFIRE 56",
>> +		"SAFFIRE PRO 40 1",
>> +	};
>> +	char model[32];
>> +	unsigned int i;
>> +	int err;
>> +
>> +	err = fw_csr_string(unit->directory, CSR_MODEL, model, sizeof(model));
>> +	if (err < 0)
>> +		return false;
>> +
>> +	for (i = 0; i < ARRAY_SIZE(models); i++) {
>> +		if (strcmp(models[i], model) == 0)
>> +			break;
>> +	}
>> +
>> +	return i < ARRAY_SIZE(models);
>> +}
> 
> The model name of my old Saffire PRO 40 is "SAFFIRE_PRO_40", and
> according to e.g. Alban Bernard on ffado-user, the newer model is
> "SAFFIRE_PRO_40_1".  Saffire 56's name is "LIQUID_SAFFIRE_56"
> according to https://forum.ubuntuusers.de/topic/focusrite-liquid/.

OK. Thanks for your correction. I'll include them in next patchset.

Well, can this patchset enable snd-dice to drive your Saffire Pro 40
with several isochronous streams?  (of cource, the strings should be fixed.)


Regards

Takashi Sakamoto

Re: [PATCH 4/4] ALSA: dice: force to add two pcm devices for listed models

[Stefan Richter]

On Mar 06 Takashi Sakamoto wrote:
> On Mar 6 2016 00:07, Stefan Richter wrote:
> > On Mar 05 Takashi Sakamoto wrote:  
> >> +static bool force_two_pcm_support(struct fw_unit *unit)
> >> +{
> >> +	const char *const models[] = {
> >> +		/* TC Electronic models. */
> >> +		"StudioKonnekt48",
> >> +		/* Focusrite models. */
> >> +		"SAFFIRE PRO 40",
> >> +		"LIQUID SAFFIRE 56",
> >> +		"SAFFIRE PRO 40 1",
> >> +	};
> >> +	char model[32];
> >> +	unsigned int i;
> >> +	int err;
> >> +
> >> +	err = fw_csr_string(unit->directory, CSR_MODEL, model, sizeof(model));
> >> +	if (err < 0)
> >> +		return false;
> >> +
> >> +	for (i = 0; i < ARRAY_SIZE(models); i++) {
> >> +		if (strcmp(models[i], model) == 0)
> >> +			break;
> >> +	}
> >> +
> >> +	return i < ARRAY_SIZE(models);
> >> +}  
> > 
> > The model name of my old Saffire PRO 40 is "SAFFIRE_PRO_40", and
> > according to e.g. Alban Bernard on ffado-user, the newer model is
> > "SAFFIRE_PRO_40_1".  Saffire 56's name is "LIQUID_SAFFIRE_56"
> > according to https://forum.ubuntuusers.de/topic/focusrite-liquid/.  
> 
> OK. Thanks for your correction. I'll include them in next patchset.
> 
> Well, can this patchset enable snd-dice to drive your Saffire Pro 40
> with several isochronous streams?  (of cource, the strings should be fixed.)

I did a first few tests on 4.5.0-rc6 now.  Hardware is Saffire Pro 40 and
a XIO2213B OHCI card.  I first cherry-picked the following patches from 
tiwai/sound.git:
ALSA: dice: limit to current sampling transfer frequency
ALSA: dice: limit stream to current sampling transfer frequency.
ALSA: dice: add MIDI ports according to current number of MIDI substreams
ALSA: dice: get the number of MBLA data channel at opening PCM substream
ALSA: dice: purge generating channel cache
ALSA: dice: ensure phase lock before starting streaming
ALSA: dice: change notification mask to detect lock status change
ALSA: dice: old firmware optimization for Dice notification
ALSA: dice: drop duplex streams synchronization to transfer own time stamps

I then applied the current 4 patches, with underscores inserted into the
model names in patch 4/4.

$ arecord -l
[...]
card 2: Pro4000dd28 [Pro40-00dd28], device 0: DICE [Pro40-00dd28]
  Subdevices: 2/2
  Subdevice #0: subdevice #0
  Subdevice #1: subdevice #1
$ aplay -l
[...]
card 2: Pro4000dd28 [Pro40-00dd28], device 0: DICE [Pro40-00dd28]
  Subdevices: 2/2
  Subdevice #0: subdevice #0
  Subdevice #1: subdevice #1

I tested capture and playback at 44100 and 48000 Hz at both subdevices,
i.e. on plughw:2,0,0 and plughw:2,0,1 and it seems to work.  I only really
checked subdevice 0 with I/O other than silence because I am not currently
prepared to ADAT.  (As you remember there are only ADAT channels on the
second stream.  Given some time I can probably arrange for another ADAT
capable device to transmit and receive via ADAT.)

Then I also tried 88200 and 96000 Hz.  This gives 16+16 channels at
subdevice #0 like it should, and capture works, but playback is mute.

I have not yet checked whether the muted playback at 88200/96000 is a
consequence of these 4 new patches, or has been introduced earlier, or has
always been the case with snd-dice.  (I haven't done many systematic tests
with the Pro 40 and snd-dice yet.  If I unload snd-dice and use FFADO,
playback at 88200/96000 works.)

Furthermore I did a few quick tests with the entire patchset and Saffire
PRO 24 (which uses only 1+1 transmitter+receiver, and consequently is
shown only as 1+1 subdevice).  I tried 48000 and 96000 Hz, and capture
and playback works at both sampling rates.  IOW the PRO 40's muting
problem is not afflicting the PRO 24.

Next thing for me to do will be to remove patches and figure out whether
a previous driver revision gives me unmuted playback at 96000 Hz on the
PRO 40. 
-- 
Stefan Richter
-======----- --== --==-
http://arcgraph.de/sr/

Re: [PATCH 4/4] ALSA: dice: force to add two pcm devices for listed models

[Stefan Richter]

On Mar 06 Stefan Richter wrote:
[...]
> Then I also tried 88200 and 96000 Hz.  This gives 16+16 channels at
> subdevice #0 like it should, and capture works, but playback is mute.
[...]
> Next thing for me to do will be to remove patches and figure out whether
> a previous driver revision gives me unmuted playback at 96000 Hz on the
> PRO 40. 

Plain v4.5-rc6 gave me unmuted playback at 88200/96000 Hz once, but it was
distorted by some sort of high clinking.  Other times plain v4.5-rc6 had
muted playback at 88200/96000 Hz too.

I tested with mplayer+jack+alsa, audacious+jack+alsa, and
audacious+alsa.
-- 
Stefan Richter
-======----- --== --===
http://arcgraph.de/sr/

Re: [PATCH 4/4] ALSA: dice: force to add two pcm devices for listed models

[Takashi Sakamoto]

Hi Stefan,

On Mar 7 2016 7:55, Stefan Richter wrote:
 > $ arecord -l
 > [...]
 > card 2: Pro4000dd28 [Pro40-00dd28], device 0: DICE [Pro40-00dd28]
 >   Subdevices: 2/2
 >   Subdevice #0: subdevice #0
 >   Subdevice #1: subdevice #1
 > $ aplay -l
 > [...]
 > card 2: Pro4000dd28 [Pro40-00dd28], device 0: DICE [Pro40-00dd28]
 >   Subdevices: 2/2
 >   Subdevice #0: subdevice #0
 >   Subdevice #1: subdevice #1
 >
 > I tested capture and playback at 44100 and 48000 Hz at both
 > subdevices, i.e. on plughw:2,0,0 and plughw:2,0,1 and it seems to
 > work.  I only really checked subdevice 0 with I/O other than silence
 > because I am not currently prepared to ADAT.  (As you remember there
 > are only ADAT channels on the second stream.  Given some time I can
 > probably arrange for another ADAT capable device to transmit and
 > receive via ADAT.)
 >
 > Then I also tried 88200 and 96000 Hz.  This gives 16+16 channels at
 > subdevice #0 like it should, and capture works, but playback is mute.
 >
 > I have not yet checked whether the muted playback at 88200/96000 is a
 > consequence of these 4 new patches, or has been introduced earlier,
 > or has always been the case with snd-dice.  (I haven't done many
 > systematic tests with the Pro 40 and snd-dice yet.  If I unload
 > snd-dice and use FFADO, playback at 88200/96000 works.)
 >
 > Furthermore I did a few quick tests with the entire patchset and
 > Saffire PRO 24 (which uses only 1+1 transmitter+receiver, and
 > consequently is shown only as 1+1 subdevice).  I tried 48000 and
 > 96000 Hz, and capture and playback works at both sampling rates.  IOW
 > the PRO 40's muting problem is not afflicting the PRO 24.

On Mar 7 2016 09:24, Stefan Richter wrote:
> On Mar 06 Stefan Richter wrote:
> [...]
>> Then I also tried 88200 and 96000 Hz.  This gives 16+16 channels at
>> subdevice #0 like it should, and capture works, but playback is mute.
> [...]
>> Next thing for me to do will be to remove patches and figure out whether
>> a previous driver revision gives me unmuted playback at 96000 Hz on the
>> PRO 40.
>
> Plain v4.5-rc6 gave me unmuted playback at 88200/96000 Hz once, but it was
> distorted by some sort of high clinking.  Other times plain v4.5-rc6 had
> muted playback at 88200/96000 Hz too.
>
> I tested with mplayer+jack+alsa, audacious+jack+alsa, and
> audacious+alsa.

Thanks for your report. Totally:
  * The patched snd-dice still works fine with Saffire Pro 24.
  * The pached snd-dice successfully manages several isochronous streams
    without code bugs (i.e. kernel NULL pointer dereference or
    continuous locking).
  * The pached snd-dice still has mute issue for Saffire Pro 40.

Anyway, the result cannot block applying this patchset for 4.6. I'll 
post it in this night for merging.


Well, about the mute issue, I have no idea to fix it at all. At least, 
for my test units, the patched snd-dice works fine without such issues. 
Therefore, it's nearly model-dependent issue and I have no way to do for 
it. FYI, I tested with:
  * OHCI 1394 host controllers
   * VIA VT6315
   * TI XIO2213
  * Dice II units
   * TC Electronic ImpactTwin
   * TC Electronic Konnekt24D
   * TC Electronic Konnekt8
* Dice Jr. units
   * TC Electronic DesktopKonnect6
* Dice Mini units
   * Focusrite Saffire Pro 26


Regards

Takashi Sakamoto

Re: [PATCH 4/4] ALSA: dice: force to add two pcm devices for listed models

[Takashi Sakamoto]

On Mar 7 2016 22:43, Stefan Richter wrote:
> On Mar 07 Takashi Sakamoto wrote:
>> Thanks for your report. Totally:
>>   * The patched snd-dice still works fine with Saffire Pro 24.
>>   * The pached snd-dice successfully manages several isochronous streams
>>     without code bugs (i.e. kernel NULL pointer dereference or
>>     continuous locking) with Saffire Pro 24/40.
>>   * The pached snd-dice still has mute issue with Saffire Pro 40.
> 
> I concur.  While I was only able to perform just a few tests yet, there
> doesn't appear to be any regression, and this patchset adds support for
> the previously inaccessible channels at the second transmitter and
> receiver.
> 
>> Anyway, the result cannot block applying this patchset for 4.6. I'll 
>> post it in this night for merging.
> 
> Thanks.

I posted. Thanks for your test ;)

>> Well, about the mute issue, I have no idea to fix it at all. At least, 
>> for my test units, the patched snd-dice works fine without such issues. 
>> Therefore, it's nearly model-dependent issue and I have no way to do for 
>> it. FYI, I tested with:
>>   * OHCI 1394 host controllers
>>    * VIA VT6315
>>    * TI XIO2213
>>   * Dice II units
>>    * TC Electronic ImpactTwin
>>    * TC Electronic Konnekt24D
>>    * TC Electronic Konnekt8
>> * Dice Jr. units
>>    * TC Electronic DesktopKonnect6
>> * Dice Mini units
>>    * Focusrite Saffire Pro 26
> 
> FWIW, Saffire PRO 24 and 40 are based on Dice Junior according to FFADO's
> device database.  If desired I could open up my 24 and 40 to check.

If you don't mind it, please.
(Because once opening your unit, usually, you cannot get repair service
from sellers.)

And I wrote wrong for Dice Jr./Mini units. Correctly:
 * Dice Jr. units
  * TC Electronic DesktopKonnect6
  * Focusrite Saffire Pro 26
 * I have no units with Dice Mini (Oh...).

Anyway, all of my units with Dice Jr. works fine. So we can judge that
Pro 40 has its own quirk which we don't know.

> Furthermore FYI, the FFADO userspace streaming driver has got the following
> two known issues with the PRO 40:
> 	http://subversion.ffado.org/ticket/326
> 	playback at 96 kHz is choppy (but fine at 44.1, 48, 88.2 kHz;
>         reproduced by multiple users, 100% reproducible by myself too)
> 	http://subversion.ffado.org/ticket/372
> 	MIDI playback problem (perhaps dependent on firmware revision;
>         cannot reproduce this myself due to lack of MIDI equipment)

Hm. With a lack of something like packet dump or detailed
case-investigation, I can do nothing for the unit. I have no fashion to
lost my time for meaningless talks, sorry.


For our information, I put your result of hinawa-dice-common-cui from
Saffire Pro 40. (meaningless lines are omitted.)

$ ./hinawa-dice-common-cui 00130e040140dd28 current-status
owner address: ffc1000100000000
global enabled: False
clock status:
  locked: 1
  nominal: 48000
clock source detection:
  locked:
  slip:
measured rate: 48000
dice version: 1.0.12.0
latest notification: 0x00000010
clock source: Internal
sampling rate: 48000
nickname: Pro40-00dd28
Transmitted streams:
  Stream 0:
    iso-channel: 4294967295
    speed:       2
    pcm:         10
    midi:        1
    iec60958:
      caps:      0
      enable:    0
    formation:
      0:         IP 1
      1:         IP 2
      2:         IP 3
      3:         IP 4
      4:         IP 5
      5:         IP 6
      6:         IP 7
      7:         IP 8
      8:         SPDIF L
      9:         SPDIF R
  Stream 1:
    iso-channel: 4294967295
    speed:       2
    pcm:         10
    midi:        0
    iec60958:
      caps:      0
      enable:    0
    formation:
      0:         ADAT 1
      1:         ADAT 2
      2:         ADAT 3
      3:         ADAT 4
      4:         ADAT 5
      5:         ADAT 6
      6:         ADAT 7
      7:         ADAT 8
      8:         Loop 1
      9:         Loop 2
Received streams:
  Stream 0:
    iso-channel: 4294967295
    start:       0
    pcm:         12
    midi:        1
    iec60958:
      caps:      0
      enable:    0
    formation:
      0:         Mon 1
      1:         Mon 2
      2:         Line 3
      3:         Line 4
      4:         Line 5
      5:         Line 6
      6:         Line 7
      7:         Line 8
      8:         Line 9
      9:         Line 10
      10:         SPDIF L
      11:         SPDIF R
  Stream 1:
    iso-channel: 4294967295
    start:       0
    pcm:         8
    midi:        0
    iec60958:
      caps:      0
      enable:    0
    formation:
      0:         ADAT 1
      1:         ADAT 2
      2:         ADAT 3
      3:         ADAT 4
      4:         ADAT 5
      5:         ADAT 6
      6:         ADAT 7
      7:         ADAT 8
Synchronization infomation:
  clock-source:  Internal
  locked:        1
  sampling-rate: 48000
    ADAT:
      data-bits: 0
      no-data:   1

$ ./hinawa-dice-common-cui 00130e040140dd28 sampling-rate set 96000

$ ./hinawa-dice-common-cui 00130e040140dd28 current-status
owner address: ffc1000100000000
global enabled: False
clock status:
  locked: 1
  nominal: 96000
clock source detection:
  locked:
  slip:
measured rate: 96000
dice version: 1.0.12.0
latest notification: 0x00000010
clock source: Internal
sampling rate: 96000
nickname: Pro40-00dd28
Transmitted streams:
  Stream 0:
    iso-channel: 4294967295
    speed:       2
    pcm:         16
    midi:        1
    iec60958:
      caps:      0
      enable:    0
    formation:
      0:         IP 1
      1:         IP 2
      2:         IP 3
      3:         IP 4
      4:         IP 5
      5:         IP 6
      6:         IP 7
      7:         IP 8
      8:         SPDIF L
      9:         SPDIF R
      10:         ADAT 1
      11:         ADAT 2
      12:         ADAT 3
      13:         ADAT 4
      14:         Loop 1
      15:         Loop 2
Received streams:
  Stream 0:
    iso-channel: 4294967295
    start:       0
    pcm:         16
    midi:        1
    iec60958:
      caps:      0
      enable:    0
    formation:
      0:         Mon 1
      1:         Mon 2
      2:         Line 3
      3:         Line 4
      4:         Line 5
      5:         Line 6
      6:         Line 7
      7:         Line 8
      8:         Line 9
      9:         Line 10
      10:         SPDIF L
      11:         SPDIF R
      12:         ADAT 1
      13:         ADAT 2
      14:         ADAT 3
      15:         ADAT 4
Synchronization infomation:
  clock-source:  Internal
  locked:        1
  sampling-rate: 96000
    ADAT:
      data-bits: 0
      no-data:   1


Regards

Takashi Sakamoto

Re: [PATCH 4/4] ALSA: dice: force to add two pcm devices for listed models

[Stefan Richter]

On Mar 07 Takashi Sakamoto wrote:
> On Mar 7 2016 22:43, Stefan Richter wrote:
> > On Mar 07 Takashi Sakamoto wrote:
> >>   * Dice II units
> >>    * TC Electronic ImpactTwin
> >>    * TC Electronic Konnekt24D
> >>    * TC Electronic Konnekt8
> >> * Dice Jr. units
> >>    * TC Electronic DesktopKonnect6
> >> * Dice Mini units
> >>    * Focusrite Saffire Pro 26  
> > 
> > FWIW, Saffire PRO 24 and 40 are based on Dice Junior according to FFADO's
> > device database.  If desired I could open up my 24 and 40 to check.  
> 
> If you don't mind it, please.
> (Because once opening your unit, usually, you cannot get repair service
> from sellers.)
> 
> And I wrote wrong for Dice Jr./Mini units. Correctly:
>  * Dice Jr. units
>   * TC Electronic DesktopKonnect6
>   * Focusrite Saffire Pro 26
>  * I have no units with Dice Mini (Oh...).
> 
> Anyway, all of my units with Dice Jr. works fine. So we can judge that
> Pro 40 has its own quirk which we don't know.

Written on the Dice chip in my Saffire PRO 24, purchased in 2010:

	TCD2210
	K34FAAB
	C0906

So that's a DICE Mini.

And the Saffire PRO 40, purchased in early 2015:

	TCD2220
	K8GVUF
	A1246

i.e. DICE JR.
-- 
Stefan Richter
-======----- -=-- -=--=
http://arcgraph.de/sr/

Re: [FFADO-devel] [PATCH 4/4] ALSA: dice: force to add two pcm devices for listed models

[Gordon Scott]

Just a reminder in case it gets overlooked.

The new Focusrite Saffire Pro 40, as of a few months ago, is now a Dice
III.

Focusrite were going to contact me again about that "after Christmas",
but despite a couple of reminders, I've heard nothing more.

Gordon.

On Sat, 2016-04-09 at 18:34 +0200, Stefan Richter wrote:
> On Mar 07 Takashi Sakamoto wrote:
> > On Mar 7 2016 22:43, Stefan Richter wrote:
> > > On Mar 07 Takashi Sakamoto wrote:
> > >>   * Dice II units
> > >>    * TC Electronic ImpactTwin
> > >>    * TC Electronic Konnekt24D
> > >>    * TC Electronic Konnekt8
> > >> * Dice Jr. units
> > >>    * TC Electronic DesktopKonnect6
> > >> * Dice Mini units
> > >>    * Focusrite Saffire Pro 26  
> > > 
> > > FWIW, Saffire PRO 24 and 40 are based on Dice Junior according to FFADO's
> > > device database.  If desired I could open up my 24 and 40 to check.  
> > 
> > If you don't mind it, please.
> > (Because once opening your unit, usually, you cannot get repair service
> > from sellers.)
> > 
> > And I wrote wrong for Dice Jr./Mini units. Correctly:
> >  * Dice Jr. units
> >   * TC Electronic DesktopKonnect6
> >   * Focusrite Saffire Pro 26
> >  * I have no units with Dice Mini (Oh...).
> > 
> > Anyway, all of my units with Dice Jr. works fine. So we can judge that
> > Pro 40 has its own quirk which we don't know.
> 
> Written on the Dice chip in my Saffire PRO 24, purchased in 2010:
> 
> 	TCD2210
> 	K34FAAB
> 	C0906
> 
> So that's a DICE Mini.
> 
> And the Saffire PRO 40, purchased in early 2015:
> 
> 	TCD2220
> 	K8GVUF
> 	A1246
> 
> i.e. DICE JR.

-- 
Gordon Scott.	Electronics and software engineering.
		Basingstoke UK
		+44-1256-476547

But for the sake of some little mouthful of flesh 
we deprive a soul of the sun and light, 
and of that proportion of life and time 
it had been born into the world to enjoy. 
=== Plutarch AD44-120

Re: [PATCH 4/4] ALSA: dice: force to add two pcm devices for listed models

[Takashi Sakamoto]

Hi Stefan,

On Apr 10 2016 01:34, Stefan Richter wrote:
> On Mar 07 Takashi Sakamoto wrote:
>> On Mar 7 2016 22:43, Stefan Richter wrote:
>>> On Mar 07 Takashi Sakamoto wrote:
>>>>   * Dice II units
>>>>    * TC Electronic ImpactTwin
>>>>    * TC Electronic Konnekt24D
>>>>    * TC Electronic Konnekt8
>>>> * Dice Jr. units
>>>>    * TC Electronic DesktopKonnect6
>>>> * Dice Mini units
>>>>    * Focusrite Saffire Pro 26  
>>>
>>> FWIW, Saffire PRO 24 and 40 are based on Dice Junior according to FFADO's
>>> device database.  If desired I could open up my 24 and 40 to check.  
>>
>> If you don't mind it, please.
>> (Because once opening your unit, usually, you cannot get repair service
>> from sellers.)
>>
>> And I wrote wrong for Dice Jr./Mini units. Correctly:
>>  * Dice Jr. units
>>   * TC Electronic DesktopKonnect6
>>   * Focusrite Saffire Pro 26
>>  * I have no units with Dice Mini (Oh...).
>>
>> Anyway, all of my units with Dice Jr. works fine. So we can judge that
>> Pro 40 has its own quirk which we don't know.
> 
> Written on the Dice chip in my Saffire PRO 24, purchased in 2010:
> 
> 	TCD2210
> 	K34FAAB
> 	C0906
> 
> So that's a DICE Mini.
> 
> And the Saffire PRO 40, purchased in early 2015:
> 
> 	TCD2220
> 	K8GVUF
> 	A1246
> 
> i.e. DICE JR.

Thanks for your confirmation. The database of FFADO project includes the
wrongs for some models, therefore it's better for us to get such evidences.

Recently I got PreSonus FireStudio Mobile from used-market. It has
TCD2210 (Dice Mini). I've started to read packet dumps for Dice ASICs
except for TCD3070-CH, on Windows/Linux (ALSA).


Well, would you please get packet dumps with your models? Especially
with SaffirePro 40 because current ALSA firewire stack cannot make it
generate correct sounds, as long as you tested. I need the dumps under
conditions below:

- From the beginning of starting packet streaming.
- During seven seconds after the beginning, at least.
- Including the contents of asynchronous packets for cycle start packet.
- Including the contents of isochronous packets for all of used
channels. At least:
 - The length of payload.
 - The number of channel
 - The node IDs for ishcchronous source and destination
 - Two quadlets of CIP header.
- A condition that your unit is driven by Windows driver.
- At both of 44.1/48.0kHz


For example, this is a sample got by my protocol analyzer (not nosy, I
have no cards with PCILynx).

rel. time (uSec),type,source ID,dest. ID,offset,label,retry
code,response code,priority,data length,quadlet
data,tag,channel,sync.,speed,acknowledge,data
0.000,CycleStart,0xFFC2,0xFFFF,0xFFFFF0000200,0,retry_1,,15,,0xE371B034,,,,100,,
2.614,Streaming,,,,,,,,8,,1,0,0,400,,0x020F0028,0x9001FFFF,0x570A5DF9
122.375,CycleStart,0xFFC2,0xFFFF,0xFFFFF0000200,0,retry_1,,15,,0xE371C034,,,,100,,
2.614,Streaming,,,,,,,,488,,1,0,0,400,,0x020F0028,0x9001DAB9, (CIP data)
122.386,CycleStart,0xFFC2,0xFFFF,0xFFFFF0000200,0,retry_1,,15,,0xE371D034,,,,100,,
2.614,Streaming,,,,,,,,488,,1,0,0,400,,0x020F0030,0x9001F423, (CIP data)
122.386,CycleStart,0xFFC2,0xFFFF,0xFFFFF0000200,0,retry_1,,15,,0xE371E034,,,,100,,
2.614,Streaming,,,,,,,,8,,1,0,0,400,,0x020F0038,0x9001FFFF,0xD4D853F5
122.386,CycleStart,0xFFC2,0xFFFF,0xFFFFF0000200,0,retry_1,,15,,0xE371F034,,,,100,,
2.614,Streaming,,,,,,,,488,,1,0,0,400,,0x020F0038,0x9001098E, (CIP data)
...

When I run my parser written by Python3, I got enough information:
['0xE371B034', '0x020F0028', '0x9001FFFF', 2795258932, 0, 0]
['0xE371C034', '0x020F0028', '0x9001DAB9', 2795262004, 2795267769, 5765]
['0xE371D034', '0x020F0030', '0x9001F423', 2795265076, 2795272227, 7151,
4458]
['0xE371E034', '0x020F0038', '0x9001FFFF', 2795268148, 0, 0]
['0xE371F034', '0x020F0038', '0x9001098E', 2795271220, 2795276686, 5466,
4459]
...

Legend:
 - CycleStart
 - CIP header 0
 - CIP header 1
 - CycleStart(tick)
 - presentation timestamp(tick)
 - subtraction between CycleStart and presentation timestamp (tick)
 - subtraction between previous/current presentation timestamp(tick)

Currently, I can find some differences between Dice ASICs:
 - The ASIC for Dice II transfers/receives packets with less
   transfer-delay than the value defined in IEC 61883-6. But packets
   for the other ASICs are compliant to it.

I don't investigate the sequence of the length of CIP data yet.


Regards

Takashi Sakamoto

Re: [PATCH 4/4] ALSA: dice: force to add two pcm devices for listed models

[Stefan Richter]

On Apr 12 Takashi Sakamoto wrote:
> would you please get packet dumps with your models? Especially
> with SaffirePro 40
[...]

I will, but it may take a while until I manage to do so.
-- 
Stefan Richter
-======----- -=-- -===-
http://arcgraph.de/sr/

[PATCH 0/4] ALSA: dice: enable to handle several streams

[Takashi Sakamoto]

Hi,

This patchset updates my previous RFT, and goes for merging.
[alsa-devel] [RFT][PATCH 0/4] ALSA: dice: enabled to handle several streams
http://mailman.alsa-project.org/pipermail/alsa-devel/2016-March/105387.html

For the previous one, Stefan Richter worked as a tester with his devices
(Focusrite Saffire Pro 24 and 40), while current patchset includes changes
related to userspace interface. Therefore, I don't add Tested-by.


In previous one, ALSA dice driver uses subdevice for userspace to access
any PCM substreams. On the other hand, the design of PCM subdevice leaves
a bit cumbersome for userspace to handle several subdevices simultaneously
in one process. These PCM substreams on a batch of AMDTP streams and it's
better for ALSA dice driver to enable one process to handle them with
simpler way.

Based on this concept, this patchset adds PCM character devices, instead of
PCM subdevice. Thus, userspace applications can utilize PCM substreams via
individual PCM character devices. In configuration space of alsa-lib, via
'hw:0,0' and 'hw:0,1' if available, like:

$ arecord -l
...
card 1: Pro26000f5a [Pro26-000f5a], device 0: DICE [Pro26-000f5a]
  Subdevices: 1/1
  Subdevice #0: subdevice #0
card 1: Pro26000f5a [Pro26-000f5a], device 1: DICE [Pro26-000f5a]
  Subdevices: 1/1
  Subdevice #0: subdevice #0


For the other parts, there's no code changes. Therefore, Stefan's result
still has meanings.

Changes:
 * Add additional PCM character device instead of PCM subdevice
 * Correct string entries to force-pcm
 * Improve comments

Takashi Sakamoto (4):
  ALSA: dice: have two sets of isochronous resources/streams
  ALSA: dice: handle whole available isochronous streams
  ALSA: dice: handle several PCM substreams when any isochronous streams
    are available
  ALSA: dice: force to add two pcm devices for listed models

 sound/firewire/dice/dice-midi.c   |   8 +-
 sound/firewire/dice/dice-pcm.c    | 147 +++++++++-----
 sound/firewire/dice/dice-stream.c | 398 ++++++++++++++++++++++++--------------
 sound/firewire/dice/dice.c        |  41 ++++
 sound/firewire/dice/dice.h        |  33 +++-
 5 files changed, 424 insertions(+), 203 deletions(-)

-- 
2.7.0

[PATCH 2/4] ALSA: dice: handle whole available isochronous streams

[Takashi Sakamoto]

This commit enables ALSA dice driver to handle whole available streams.

In Dice, certain registers represent the number of available streams at
current sampling transfer frequency for both directions. The parameters
of each stream are represented in a block of register. This block is
aligned sequentially. These streams start simultaneously by writing
enable bit to a register.

This commit operates these registers when starting/stopping streams.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/dice/dice-stream.c | 398 ++++++++++++++++++++++++--------------
 1 file changed, 255 insertions(+), 143 deletions(-)

diff --git a/sound/firewire/dice/dice-stream.c b/sound/firewire/dice/dice-stream.c
index 15d581d..2077f18 100644
--- a/sound/firewire/dice/dice-stream.c
+++ b/sound/firewire/dice/dice-stream.c
@@ -65,85 +65,84 @@ static int ensure_phase_lock(struct snd_dice *dice)
 	return 0;
 }
 
-static void release_resources(struct snd_dice *dice,
-			      struct fw_iso_resources *resources)
+static int get_register_params(struct snd_dice *dice, unsigned int params[4])
 {
-	__be32 channel;
-
-	/* Reset channel number */
-	channel = cpu_to_be32((u32)-1);
-	if (resources == &dice->tx_resources[0])
-		snd_dice_transaction_write_tx(dice, TX_ISOCHRONOUS,
-					      &channel, sizeof(channel));
-	else
-		snd_dice_transaction_write_rx(dice, RX_ISOCHRONOUS,
-					      &channel, sizeof(channel));
-
-	fw_iso_resources_free(resources);
-}
-
-static int keep_resources(struct snd_dice *dice,
-			  struct fw_iso_resources *resources,
-			  unsigned int max_payload_bytes)
-{
-	__be32 channel;
+	__be32 reg[2];
 	int err;
 
-	err = fw_iso_resources_allocate(resources, max_payload_bytes,
-				fw_parent_device(dice->unit)->max_speed);
+	err = snd_dice_transaction_read_tx(dice, TX_NUMBER, reg, sizeof(reg));
 	if (err < 0)
-		goto end;
+		return err;
+	params[0] = min_t(unsigned int, be32_to_cpu(reg[0]), MAX_STREAMS);
+	params[1] = be32_to_cpu(reg[1]) * 4;
 
-	/* Set channel number */
-	channel = cpu_to_be32(resources->channel);
-	if (resources == &dice->tx_resources[0])
-		err = snd_dice_transaction_write_tx(dice, TX_ISOCHRONOUS,
-						    &channel, sizeof(channel));
-	else
-		err = snd_dice_transaction_write_rx(dice, RX_ISOCHRONOUS,
-						    &channel, sizeof(channel));
+	err = snd_dice_transaction_read_rx(dice, RX_NUMBER, reg, sizeof(reg));
 	if (err < 0)
-		release_resources(dice, resources);
-end:
-	return err;
+		return err;
+	params[2] = min_t(unsigned int, be32_to_cpu(reg[0]), MAX_STREAMS);
+	params[3] = be32_to_cpu(reg[1]) * 4;
+
+	return 0;
 }
 
-static void stop_stream(struct snd_dice *dice, struct amdtp_stream *stream)
+static void release_resources(struct snd_dice *dice)
 {
-	amdtp_stream_pcm_abort(stream);
-	amdtp_stream_stop(stream);
+	unsigned int i;
+
+	for (i = 0; i < MAX_STREAMS; i++) {
+		if (amdtp_stream_running(&dice->tx_stream[i])) {
+			amdtp_stream_pcm_abort(&dice->tx_stream[i]);
+			amdtp_stream_stop(&dice->tx_stream[i]);
+		}
+		if (amdtp_stream_running(&dice->rx_stream[i])) {
+			amdtp_stream_pcm_abort(&dice->rx_stream[i]);
+			amdtp_stream_stop(&dice->rx_stream[i]);
+		}
 
-	if (stream == &dice->tx_stream[0])
-		release_resources(dice, &dice->tx_resources[0]);
-	else
-		release_resources(dice, &dice->rx_resources[0]);
+		fw_iso_resources_free(&dice->tx_resources[i]);
+		fw_iso_resources_free(&dice->rx_resources[i]);
+	}
 }
 
-static int start_stream(struct snd_dice *dice, struct amdtp_stream *stream,
-			unsigned int rate)
+static void stop_streams(struct snd_dice *dice, enum amdtp_stream_direction dir,
+			 unsigned int count, unsigned int size)
 {
+	__be32 reg;
+	unsigned int i;
+
+	for (i = 0; i < count; i++) {
+		reg = cpu_to_be32((u32)-1);
+		if (dir == AMDTP_IN_STREAM) {
+			snd_dice_transaction_write_tx(dice,
+						size * i + TX_ISOCHRONOUS,
+						&reg, sizeof(reg));
+		} else {
+			snd_dice_transaction_write_rx(dice,
+						size * i + RX_ISOCHRONOUS,
+						&reg, sizeof(reg));
+		}
+	}
+}
+
+static int keep_resources(struct snd_dice *dice,
+			  enum amdtp_stream_direction dir, unsigned int index,
+			  unsigned int rate, unsigned int pcm_chs,
+			  unsigned int midi_ports)
+{
+	struct amdtp_stream *stream;
 	struct fw_iso_resources *resources;
-	__be32 reg[2];
-	unsigned int i, pcm_chs, midi_ports;
 	bool double_pcm_frames;
+	unsigned int i;
 	int err;
 
-	if (stream == &dice->tx_stream[0]) {
-		resources = &dice->tx_resources[0];
-		err = snd_dice_transaction_read_tx(dice, TX_NUMBER_AUDIO,
-						   reg, sizeof(reg));
+	if (dir == AMDTP_IN_STREAM) {
+		stream = &dice->tx_stream[index];
+		resources = &dice->tx_resources[index];
 	} else {
-		resources = &dice->rx_resources[0];
-		err = snd_dice_transaction_read_rx(dice, RX_NUMBER_AUDIO,
-						   reg, sizeof(reg));
+		stream = &dice->rx_stream[index];
+		resources = &dice->rx_resources[index];
 	}
 
-	if (err < 0)
-		goto end;
-
-	pcm_chs = be32_to_cpu(reg[0]);
-	midi_ports = be32_to_cpu(reg[1]);
-
 	/*
 	 * At 176.4/192.0 kHz, Dice has a quirk to transfer two PCM frames in
 	 * one data block of AMDTP packet. Thus sampling transfer frequency is
@@ -163,7 +162,7 @@ static int start_stream(struct snd_dice *dice, struct amdtp_stream *stream,
 	err = amdtp_am824_set_parameters(stream, rate, pcm_chs, midi_ports,
 					 double_pcm_frames);
 	if (err < 0)
-		goto end;
+		return err;
 
 	if (double_pcm_frames) {
 		pcm_chs /= 2;
@@ -175,122 +174,208 @@ static int start_stream(struct snd_dice *dice, struct amdtp_stream *stream,
 		}
 	}
 
-	err = keep_resources(dice, resources,
-			     amdtp_stream_get_max_payload(stream));
-	if (err < 0) {
-		dev_err(&dice->unit->device,
-			"fail to keep isochronous resources\n");
-		goto end;
+	return fw_iso_resources_allocate(resources,
+				amdtp_stream_get_max_payload(stream),
+				fw_parent_device(dice->unit)->max_speed);
+}
+
+static int start_streams(struct snd_dice *dice, enum amdtp_stream_direction dir,
+			 unsigned int rate, unsigned int count,
+			 unsigned int size)
+{
+	__be32 reg[2];
+	unsigned int i, pcm_chs, midi_ports;
+	struct amdtp_stream *streams;
+	struct fw_iso_resources *resources;
+	int err = 0;
+
+	if (dir == AMDTP_IN_STREAM) {
+		streams = dice->tx_stream;
+		resources = dice->tx_resources;
+	} else {
+		streams = dice->rx_stream;
+		resources = dice->rx_resources;
+	}
+
+	for (i = 0; i < count; i++) {
+		if (dir == AMDTP_IN_STREAM) {
+			err = snd_dice_transaction_read_tx(dice,
+						size * i + TX_NUMBER_AUDIO,
+						reg, sizeof(reg));
+		} else {
+			err = snd_dice_transaction_read_rx(dice,
+						size * i + RX_NUMBER_AUDIO,
+						reg, sizeof(reg));
+		}
+		if (err < 0)
+			return err;
+		pcm_chs = be32_to_cpu(reg[0]);
+		midi_ports = be32_to_cpu(reg[1]);
+
+		err = keep_resources(dice, dir, i, rate, pcm_chs, midi_ports);
+		if (err < 0)
+			return err;
+
+		reg[0] = cpu_to_be32(resources[i].channel);
+		if (dir == AMDTP_IN_STREAM) {
+			err = snd_dice_transaction_write_tx(dice,
+						size * i + TX_ISOCHRONOUS,
+						reg, sizeof(reg[0]));
+		} else {
+			err = snd_dice_transaction_write_rx(dice,
+						size * i + RX_ISOCHRONOUS,
+						reg, sizeof(reg[0]));
+		}
+		if (err < 0)
+			return err;
+
+		err = amdtp_stream_start(&streams[i], resources[i].channel,
+				fw_parent_device(dice->unit)->max_speed);
+		if (err < 0)
+			return err;
 	}
 
-	err = amdtp_stream_start(stream, resources->channel,
-				 fw_parent_device(dice->unit)->max_speed);
-	if (err < 0)
-		release_resources(dice, resources);
-end:
 	return err;
 }
 
+/*
+ * MEMO: After this function, there're two states of streams:
+ *  - None streams are running.
+ *  - All streams are running.
+ */
 int snd_dice_stream_start_duplex(struct snd_dice *dice, unsigned int rate)
 {
-	struct amdtp_stream *master, *slave;
 	unsigned int curr_rate;
-	int err = 0;
+	unsigned int i;
+	unsigned int reg_params[4];
+	bool need_to_start;
+	int err;
 
 	if (dice->substreams_counter == 0)
-		goto end;
-
-	master = &dice->rx_stream[0];
-	slave  = &dice->tx_stream[0];
+		return -EIO;
 
-	/* Some packet queueing errors. */
-	if (amdtp_streaming_error(master) || amdtp_streaming_error(slave))
-		stop_stream(dice, master);
+	err = get_register_params(dice, reg_params);
+	if (err < 0)
+		return err;
 
-	/* Stop stream if rate is different. */
 	err = snd_dice_transaction_get_rate(dice, &curr_rate);
 	if (err < 0) {
 		dev_err(&dice->unit->device,
 			"fail to get sampling rate\n");
-		goto end;
+		return err;
 	}
 	if (rate == 0)
 		rate = curr_rate;
-	if (rate != curr_rate) {
-		err = -EINVAL;
-		goto end;
+	if (rate != curr_rate)
+		return -EINVAL;
+
+	/* Judge to need to restart streams. */
+	for (i = 0; i < MAX_STREAMS; i++) {
+		if (i < reg_params[0]) {
+			if (amdtp_streaming_error(&dice->tx_stream[i]) ||
+			    !amdtp_stream_running(&dice->tx_stream[i]))
+				break;
+		}
+		if (i < reg_params[2]) {
+			if (amdtp_streaming_error(&dice->rx_stream[i]) ||
+			    !amdtp_stream_running(&dice->rx_stream[i]))
+				break;
+		}
 	}
+	need_to_start = (i < MAX_STREAMS);
 
-	if (!amdtp_stream_running(master)) {
-		stop_stream(dice, slave);
+	if (need_to_start) {
+		/* Stop transmission. */
 		snd_dice_transaction_clear_enable(dice);
+		stop_streams(dice, AMDTP_IN_STREAM, reg_params[0],
+			     reg_params[1]);
+		stop_streams(dice, AMDTP_OUT_STREAM, reg_params[2],
+			     reg_params[3]);
+		release_resources(dice);
 
 		err = ensure_phase_lock(dice);
 		if (err < 0) {
 			dev_err(&dice->unit->device,
 				"fail to ensure phase lock\n");
-			goto end;
+			return err;
 		}
 
 		/* Start both streams. */
-		err = start_stream(dice, master, rate);
-		if (err < 0) {
-			dev_err(&dice->unit->device,
-				"fail to start AMDTP master stream\n");
-			goto end;
-		}
-		err = start_stream(dice, slave, rate);
-		if (err < 0) {
-			dev_err(&dice->unit->device,
-				"fail to start AMDTP slave stream\n");
-			stop_stream(dice, master);
-			goto end;
-		}
+		err = start_streams(dice, AMDTP_IN_STREAM, rate, reg_params[0],
+				    reg_params[1]);
+		if (err < 0)
+			goto error;
+		err = start_streams(dice, AMDTP_OUT_STREAM, rate, reg_params[2],
+				    reg_params[3]);
+		if (err < 0)
+			goto error;
+
 		err = snd_dice_transaction_set_enable(dice);
 		if (err < 0) {
 			dev_err(&dice->unit->device,
 				"fail to enable interface\n");
-			stop_stream(dice, master);
-			stop_stream(dice, slave);
-			goto end;
+			goto error;
 		}
 
-		/* Wait first callbacks */
-		if (!amdtp_stream_wait_callback(master, CALLBACK_TIMEOUT) ||
-		    !amdtp_stream_wait_callback(slave, CALLBACK_TIMEOUT)) {
-			snd_dice_transaction_clear_enable(dice);
-			stop_stream(dice, master);
-			stop_stream(dice, slave);
-			err = -ETIMEDOUT;
+		for (i = 0; i < MAX_STREAMS; i++) {
+			if ((i < reg_params[0] &&
+			    !amdtp_stream_wait_callback(&dice->tx_stream[i],
+							CALLBACK_TIMEOUT)) ||
+			    (i < reg_params[2] &&
+			     !amdtp_stream_wait_callback(&dice->rx_stream[i],
+							 CALLBACK_TIMEOUT))) {
+				err = -ETIMEDOUT;
+				goto error;
+			}
 		}
 	}
-end:
+
+	return err;
+error:
+	snd_dice_transaction_clear_enable(dice);
+	stop_streams(dice, AMDTP_IN_STREAM, reg_params[0], reg_params[1]);
+	stop_streams(dice, AMDTP_OUT_STREAM, reg_params[2], reg_params[3]);
+	release_resources(dice);
 	return err;
 }
 
+/*
+ * MEMO: After this function, there're two states of streams:
+ *  - None streams are running.
+ *  - All streams are running.
+ */
 void snd_dice_stream_stop_duplex(struct snd_dice *dice)
 {
+	unsigned int reg_params[4];
+
 	if (dice->substreams_counter > 0)
 		return;
 
 	snd_dice_transaction_clear_enable(dice);
 
-	stop_stream(dice, &dice->tx_stream[0]);
-	stop_stream(dice, &dice->rx_stream[0]);
+	if (get_register_params(dice, reg_params) == 0) {
+		stop_streams(dice, AMDTP_IN_STREAM, reg_params[0],
+			     reg_params[1]);
+		stop_streams(dice, AMDTP_OUT_STREAM, reg_params[2],
+			     reg_params[3]);
+	}
+
+	release_resources(dice);
 }
 
-static int init_stream(struct snd_dice *dice, struct amdtp_stream *stream)
+static int init_stream(struct snd_dice *dice, enum amdtp_stream_direction dir,
+		       unsigned int index)
 {
-	int err;
+	struct amdtp_stream *stream;
 	struct fw_iso_resources *resources;
-	enum amdtp_stream_direction dir;
+	int err;
 
-	if (stream == &dice->tx_stream[0]) {
-		resources = &dice->tx_resources[0];
-		dir = AMDTP_IN_STREAM;
+	if (dir == AMDTP_IN_STREAM) {
+		stream = &dice->tx_stream[index];
+		resources = &dice->tx_resources[index];
 	} else {
-		resources = &dice->rx_resources[0];
-		dir = AMDTP_OUT_STREAM;
+		stream = &dice->rx_stream[index];
+		resources = &dice->rx_resources[index];
 	}
 
 	err = fw_iso_resources_init(resources, dice->unit);
@@ -311,14 +396,20 @@ end:
  * This function should be called before starting streams or after stopping
  * streams.
  */
-static void destroy_stream(struct snd_dice *dice, struct amdtp_stream *stream)
+static void destroy_stream(struct snd_dice *dice,
+			   enum amdtp_stream_direction dir,
+			   unsigned int index)
 {
+	struct amdtp_stream *stream;
 	struct fw_iso_resources *resources;
 
-	if (stream == &dice->tx_stream[0])
-		resources = &dice->tx_resources[0];
-	else
-		resources = &dice->rx_resources[0];
+	if (dir == AMDTP_IN_STREAM) {
+		stream = &dice->tx_stream[index];
+		resources = &dice->tx_resources[index];
+	} else {
+		stream = &dice->rx_stream[index];
+		resources = &dice->rx_resources[index];
+	}
 
 	amdtp_stream_destroy(stream);
 	fw_iso_resources_destroy(resources);
@@ -326,33 +417,53 @@ static void destroy_stream(struct snd_dice *dice, struct amdtp_stream *stream)
 
 int snd_dice_stream_init_duplex(struct snd_dice *dice)
 {
-	int err;
+	int i, err;
 
-	dice->substreams_counter = 0;
-
-	err = init_stream(dice, &dice->tx_stream[0]);
-	if (err < 0)
-		goto end;
+	for (i = 0; i < MAX_STREAMS; i++) {
+		err = init_stream(dice, AMDTP_IN_STREAM, i);
+		if (err < 0) {
+			for (; i >= 0; i--)
+				destroy_stream(dice, AMDTP_OUT_STREAM, i);
+			goto end;
+		}
+	}
 
-	err = init_stream(dice, &dice->rx_stream[0]);
-	if (err < 0)
-		destroy_stream(dice, &dice->tx_stream[0]);
+	for (i = 0; i < MAX_STREAMS; i++) {
+		err = init_stream(dice, AMDTP_OUT_STREAM, i);
+		if (err < 0) {
+			for (; i >= 0; i--)
+				destroy_stream(dice, AMDTP_OUT_STREAM, i);
+			for (i = 0; i < MAX_STREAMS; i++)
+				destroy_stream(dice, AMDTP_IN_STREAM, i);
+			break;
+		}
+	}
 end:
 	return err;
 }
 
 void snd_dice_stream_destroy_duplex(struct snd_dice *dice)
 {
+	unsigned int reg_params[4];
+
 	snd_dice_transaction_clear_enable(dice);
 
-	destroy_stream(dice, &dice->tx_stream[0]);
-	destroy_stream(dice, &dice->rx_stream[0]);
+	if (get_register_params(dice, reg_params) == 0) {
+		stop_streams(dice, AMDTP_IN_STREAM, reg_params[0],
+			     reg_params[1]);
+		stop_streams(dice, AMDTP_OUT_STREAM, reg_params[2],
+			     reg_params[3]);
+	}
+
+	release_resources(dice);
 
 	dice->substreams_counter = 0;
 }
 
 void snd_dice_stream_update_duplex(struct snd_dice *dice)
 {
+	unsigned int reg_params[4];
+
 	/*
 	 * On a bus reset, the DICE firmware disables streaming and then goes
 	 * off contemplating its own navel for hundreds of milliseconds before
@@ -363,11 +474,12 @@ void snd_dice_stream_update_duplex(struct snd_dice *dice)
 	 */
 	dice->global_enabled = false;
 
-	stop_stream(dice, &dice->rx_stream[0]);
-	stop_stream(dice, &dice->tx_stream[0]);
-
-	fw_iso_resources_update(&dice->rx_resources[0]);
-	fw_iso_resources_update(&dice->tx_resources[0]);
+	if (get_register_params(dice, reg_params) == 0) {
+		stop_streams(dice, AMDTP_IN_STREAM, reg_params[0],
+			     reg_params[1]);
+		stop_streams(dice, AMDTP_OUT_STREAM, reg_params[2],
+			     reg_params[3]);
+	}
 }
 
 static void dice_lock_changed(struct snd_dice *dice)
-- 
2.7.0

Re: [PATCH 2/4] ALSA: dice: handle whole available isochronous streams

[Takashi Iwai]

On Mon, 07 Mar 2016 14:35:43 +0100,
Takashi Sakamoto wrote:
> 
> This commit enables ALSA dice driver to handle whole available streams.
> 
> In Dice, certain registers represent the number of available streams at
> current sampling transfer frequency for both directions. The parameters
> of each stream are represented in a block of register. This block is
> aligned sequentially. These streams start simultaneously by writing
> enable bit to a register.
> 
> This commit operates these registers when starting/stopping streams.
> 
> Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
> ---
>  sound/firewire/dice/dice-stream.c | 398 ++++++++++++++++++++++++--------------
>  1 file changed, 255 insertions(+), 143 deletions(-)
> 
> diff --git a/sound/firewire/dice/dice-stream.c b/sound/firewire/dice/dice-stream.c
> index 15d581d..2077f18 100644
> --- a/sound/firewire/dice/dice-stream.c
> +++ b/sound/firewire/dice/dice-stream.c
> @@ -65,85 +65,84 @@ static int ensure_phase_lock(struct snd_dice *dice)
>  	return 0;
>  }
>  
> -static void release_resources(struct snd_dice *dice,
> -			      struct fw_iso_resources *resources)
> +static int get_register_params(struct snd_dice *dice, unsigned int params[4])
>  {
> -	__be32 channel;
> -
> -	/* Reset channel number */
> -	channel = cpu_to_be32((u32)-1);
> -	if (resources == &dice->tx_resources[0])
> -		snd_dice_transaction_write_tx(dice, TX_ISOCHRONOUS,
> -					      &channel, sizeof(channel));
> -	else
> -		snd_dice_transaction_write_rx(dice, RX_ISOCHRONOUS,
> -					      &channel, sizeof(channel));
> -
> -	fw_iso_resources_free(resources);
> -}
> -
> -static int keep_resources(struct snd_dice *dice,
> -			  struct fw_iso_resources *resources,
> -			  unsigned int max_payload_bytes)
> -{
> -	__be32 channel;
> +	__be32 reg[2];
>  	int err;
>  
> -	err = fw_iso_resources_allocate(resources, max_payload_bytes,
> -				fw_parent_device(dice->unit)->max_speed);
> +	err = snd_dice_transaction_read_tx(dice, TX_NUMBER, reg, sizeof(reg));
>  	if (err < 0)
> -		goto end;
> +		return err;
> +	params[0] = min_t(unsigned int, be32_to_cpu(reg[0]), MAX_STREAMS);
> +	params[1] = be32_to_cpu(reg[1]) * 4;
>  
> -	/* Set channel number */
> -	channel = cpu_to_be32(resources->channel);
> -	if (resources == &dice->tx_resources[0])
> -		err = snd_dice_transaction_write_tx(dice, TX_ISOCHRONOUS,
> -						    &channel, sizeof(channel));
> -	else
> -		err = snd_dice_transaction_write_rx(dice, RX_ISOCHRONOUS,
> -						    &channel, sizeof(channel));
> +	err = snd_dice_transaction_read_rx(dice, RX_NUMBER, reg, sizeof(reg));
>  	if (err < 0)
> -		release_resources(dice, resources);
> -end:
> -	return err;
> +		return err;
> +	params[2] = min_t(unsigned int, be32_to_cpu(reg[0]), MAX_STREAMS);
> +	params[3] = be32_to_cpu(reg[1]) * 4;

These "parameters" are not necessarily an array since each field has a
strict definition.  A more readable code would be to define a struct
instead.


thanks,

Takashi