[PATCH 00/12] ALSA: firewire-digi00x/dice: reserve/release isochronous resources in pcm.hw_params/hw_free callbacks

[PATCH 00/12] ALSA: firewire-digi00x/dice: reserve/release isochronous resources in pcm.hw_params/hw_free callbacks

[Takashi Sakamoto]

Hi,

This patchset is a part of series of patches for all of drivers in
ALSA firewire stack to reserve/release isochronous resources in
pcm.hw_params/hw_free callbacks.

In current implementation, the resources are reserved at the same time
to start packet streaming, and released at the same time to stop packet
streaming. However, once allocated, the resources are available
independent of lifetime of each of packet streaming.

The isochronous resources are the resources of IEEE 1394 bus. On the
other side of view, it's a kind of resources of hardware to maintain
the bus (isochronous resource manager). For this kind of reservation and
release, hw_params and hw_free operations are suitable in ALSA PCM
interface.

Ideally, the operation to reserve/release isochronous resource should
be separated from the operation to start/stop packet streaming. However,
IEEE 1394 bus has reset event. Once reset occurs, isochronous resource
manager releases allocated resources. The resources should be
reallocated by requesters themselves. For this reason, in this patchset,
bus generation is checked before starting packet streaming. If
generation is updated, reallocation is requested to isochronous
resource manager, then packet streaming starts.

Takashi Sakamoto (12):
  ALSA: firewire-digi00x: refactoring to move timing of registration for
    isochronous channel
  ALSA: firewire-digi00x: code refactoring to finish streaming session
  ALSA: firewire-digi00x: simplify error path to begin streaming session
  ALSA: firewire-digi00x: code refactoring to keep isochronous resources
  ALSA: firewire-digi00x: reserve/release isochronous resources in
    pcm.hw_params/hw_free callbacks
  ALSA: firewire-digi00x: update isochronous resources when starting
    packet streaming after bus-reset
  ALSA: firewire-digi00x: code refactoring for pcm.hw_params/hw_free
    callbacks
  ALSA: dice: code refactoring to stop packet streaming
  ALSA: dice: code refactoring to keep isochronous resources
  ALSA: dice: reserve/release isochronous resources in
    pcm.hw_params/hw_free callbacks
  ALSA: dice: update isochronous resources when starting packet
    streaming after bus-reset
  ALSA: dice: code refactoring for pcm.hw_params/hw_free callbacks

 sound/firewire/dice/dice-midi.c         |  10 +-
 sound/firewire/dice/dice-pcm.c          |  62 ++---
 sound/firewire/dice/dice-stream.c       | 350 +++++++++++++-----------
 sound/firewire/dice/dice.h              |   4 +-
 sound/firewire/digi00x/digi00x-midi.c   |  10 +-
 sound/firewire/digi00x/digi00x-pcm.c    |  65 ++---
 sound/firewire/digi00x/digi00x-stream.c | 189 ++++++-------
 sound/firewire/digi00x/digi00x.h        |   4 +-
 8 files changed, 342 insertions(+), 352 deletions(-)

-- 
2.20.1

[PATCH 01/12] ALSA: firewire-digi00x: refactoring to move timing of registration for isochronous channel

[Takashi Sakamoto]

This commit is a part of preparation to perform allocation/release
of isochronous resources in pcm.hw_params/hw_free callbacks.

The registration of isochronous channels is done just after allocation
of isochronous resources. This commit separates the registration just
before starting packet streaming.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/digi00x/digi00x-stream.c | 41 ++++++++++++-------------
 1 file changed, 19 insertions(+), 22 deletions(-)

diff --git a/sound/firewire/digi00x/digi00x-stream.c b/sound/firewire/digi00x/digi00x-stream.c
index 4d3b4ebbdd49..455c43e81850 100644
--- a/sound/firewire/digi00x/digi00x-stream.c
+++ b/sound/firewire/digi00x/digi00x-stream.c
@@ -130,6 +130,12 @@ static void finish_session(struct snd_dg00x *dg00x)
 	snd_fw_transaction(dg00x->unit, TCODE_WRITE_QUADLET_REQUEST,
 			   DG00X_ADDR_BASE + DG00X_OFFSET_STREAMING_SET,
 			   &data, sizeof(data), 0);
+
+	// Unregister isochronous channels for both direction.
+	data = 0;
+	snd_fw_transaction(dg00x->unit, TCODE_WRITE_QUADLET_REQUEST,
+			   DG00X_ADDR_BASE + DG00X_OFFSET_ISOC_CHANNELS,
+			   &data, sizeof(data), 0);
 }
 
 static int begin_session(struct snd_dg00x *dg00x)
@@ -138,6 +144,15 @@ static int begin_session(struct snd_dg00x *dg00x)
 	u32 curr;
 	int err;
 
+	// Register isochronous channels for both direction.
+	data = cpu_to_be32((dg00x->tx_resources.channel << 16) |
+			   dg00x->rx_resources.channel);
+	err = snd_fw_transaction(dg00x->unit, TCODE_WRITE_QUADLET_REQUEST,
+				 DG00X_ADDR_BASE + DG00X_OFFSET_ISOC_CHANNELS,
+				 &data, sizeof(data), 0);
+	if (err < 0)
+		goto error;
+
 	err = snd_fw_transaction(dg00x->unit, TCODE_READ_QUADLET_REQUEST,
 				 DG00X_ADDR_BASE + DG00X_OFFSET_STREAMING_STATE,
 				 &data, sizeof(data), 0);
@@ -171,13 +186,6 @@ static int begin_session(struct snd_dg00x *dg00x)
 
 static void release_resources(struct snd_dg00x *dg00x)
 {
-	__be32 data = 0;
-
-	/* Unregister isochronous channels for both direction. */
-	snd_fw_transaction(dg00x->unit, TCODE_WRITE_QUADLET_REQUEST,
-			   DG00X_ADDR_BASE + DG00X_OFFSET_ISOC_CHANNELS,
-			   &data, sizeof(data), 0);
-
 	/* Release isochronous resources. */
 	fw_iso_resources_free(&dg00x->tx_resources);
 	fw_iso_resources_free(&dg00x->rx_resources);
@@ -186,7 +194,6 @@ static void release_resources(struct snd_dg00x *dg00x)
 static int keep_resources(struct snd_dg00x *dg00x, unsigned int rate)
 {
 	unsigned int i;
-	__be32 data;
 	int err;
 
 	/* Check sampling rate. */
@@ -216,22 +223,12 @@ static int keep_resources(struct snd_dg00x *dg00x, unsigned int rate)
 	err = fw_iso_resources_allocate(&dg00x->tx_resources,
 				amdtp_stream_get_max_payload(&dg00x->tx_stream),
 				fw_parent_device(dg00x->unit)->max_speed);
-	if (err < 0)
-		goto error;
-
-	/* Register isochronous channels for both direction. */
-	data = cpu_to_be32((dg00x->tx_resources.channel << 16) |
-			   dg00x->rx_resources.channel);
-	err = snd_fw_transaction(dg00x->unit, TCODE_WRITE_QUADLET_REQUEST,
-				 DG00X_ADDR_BASE + DG00X_OFFSET_ISOC_CHANNELS,
-				 &data, sizeof(data), 0);
-	if (err < 0)
-		goto error;
+	if (err < 0) {
+		fw_iso_resources_free(&dg00x->rx_resources);
+		return err;
+	}
 
 	return 0;
-error:
-	release_resources(dg00x);
-	return err;
 }
 
 int snd_dg00x_stream_init_duplex(struct snd_dg00x *dg00x)
-- 
2.20.1

[PATCH 02/12] ALSA: firewire-digi00x: code refactoring to finish streaming session

[Takashi Sakamoto]

This commit is a part of preparation to perform allocation/release
of isochronous resources in pcm.hw_params/hw_free callbacks.

The operation to finish packet streaming corresponds to stopping
isochronous contexts. This commit applies code refactoring to
move codes to stop into a helper function to finish the session.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/digi00x/digi00x-stream.c | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/sound/firewire/digi00x/digi00x-stream.c b/sound/firewire/digi00x/digi00x-stream.c
index 455c43e81850..90e31b63ac2f 100644
--- a/sound/firewire/digi00x/digi00x-stream.c
+++ b/sound/firewire/digi00x/digi00x-stream.c
@@ -125,8 +125,12 @@ int snd_dg00x_stream_get_external_rate(struct snd_dg00x *dg00x,
 
 static void finish_session(struct snd_dg00x *dg00x)
 {
-	__be32 data = cpu_to_be32(0x00000003);
+	__be32 data;
+
+	amdtp_stream_stop(&dg00x->tx_stream);
+	amdtp_stream_stop(&dg00x->rx_stream);
 
+	data = cpu_to_be32(0x00000003);
 	snd_fw_transaction(dg00x->unit, TCODE_WRITE_QUADLET_REQUEST,
 			   DG00X_ADDR_BASE + DG00X_OFFSET_STREAMING_SET,
 			   &data, sizeof(data), 0);
@@ -136,6 +140,10 @@ static void finish_session(struct snd_dg00x *dg00x)
 	snd_fw_transaction(dg00x->unit, TCODE_WRITE_QUADLET_REQUEST,
 			   DG00X_ADDR_BASE + DG00X_OFFSET_ISOC_CHANNELS,
 			   &data, sizeof(data), 0);
+
+	// Just after finishing the session, the device may lost transmitting
+	// functionality for a short time.
+	msleep(50);
 }
 
 static int begin_session(struct snd_dg00x *dg00x)
@@ -289,8 +297,6 @@ int snd_dg00x_stream_start_duplex(struct snd_dg00x *dg00x, unsigned int rate)
 	    amdtp_streaming_error(&dg00x->rx_stream)) {
 		finish_session(dg00x);
 
-		amdtp_stream_stop(&dg00x->tx_stream);
-		amdtp_stream_stop(&dg00x->rx_stream);
 		release_resources(dg00x);
 	}
 
@@ -346,8 +352,6 @@ int snd_dg00x_stream_start_duplex(struct snd_dg00x *dg00x, unsigned int rate)
 error:
 	finish_session(dg00x);
 
-	amdtp_stream_stop(&dg00x->tx_stream);
-	amdtp_stream_stop(&dg00x->rx_stream);
 	release_resources(dg00x);
 
 	return err;
@@ -358,16 +362,8 @@ void snd_dg00x_stream_stop_duplex(struct snd_dg00x *dg00x)
 	if (dg00x->substreams_counter > 0)
 		return;
 
-	amdtp_stream_stop(&dg00x->tx_stream);
-	amdtp_stream_stop(&dg00x->rx_stream);
 	finish_session(dg00x);
 	release_resources(dg00x);
-
-	/*
-	 * Just after finishing the session, the device may lost transmitting
-	 * functionality for a short time.
-	 */
-	msleep(50);
 }
 
 void snd_dg00x_stream_update_duplex(struct snd_dg00x *dg00x)
-- 
2.20.1

[PATCH 03/12] ALSA: firewire-digi00x: simplify error path to begin streaming session

[Takashi Sakamoto]

This commit is a part of preparation to perform allocation/release
of isochronous resources in pcm.hw_params/hw_free callbacks.

The caller of begin_session() calls finish_session() in its error path,
thus no need to call finish_session() in error path of begin_session().

This commit simplifies error path of begin_session().

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/digi00x/digi00x-stream.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/sound/firewire/digi00x/digi00x-stream.c b/sound/firewire/digi00x/digi00x-stream.c
index 90e31b63ac2f..8104af94aed5 100644
--- a/sound/firewire/digi00x/digi00x-stream.c
+++ b/sound/firewire/digi00x/digi00x-stream.c
@@ -159,13 +159,13 @@ static int begin_session(struct snd_dg00x *dg00x)
 				 DG00X_ADDR_BASE + DG00X_OFFSET_ISOC_CHANNELS,
 				 &data, sizeof(data), 0);
 	if (err < 0)
-		goto error;
+		return err;
 
 	err = snd_fw_transaction(dg00x->unit, TCODE_READ_QUADLET_REQUEST,
 				 DG00X_ADDR_BASE + DG00X_OFFSET_STREAMING_STATE,
 				 &data, sizeof(data), 0);
 	if (err < 0)
-		goto error;
+		return err;
 	curr = be32_to_cpu(data);
 
 	if (curr == 0)
@@ -180,15 +180,12 @@ static int begin_session(struct snd_dg00x *dg00x)
 					 DG00X_OFFSET_STREAMING_SET,
 					 &data, sizeof(data), 0);
 		if (err < 0)
-			goto error;
+			break;
 
 		msleep(20);
 		curr--;
 	}
 
-	return 0;
-error:
-	finish_session(dg00x);
 	return err;
 }
 
-- 
2.20.1

[PATCH 04/12] ALSA: firewire-digi00x: code refactoring to keep isochronous resources

[Takashi Sakamoto]

This commit is a part of preparation to perform allocation/release
of isochronous resources in pcm.hw_params/hw_free callbacks.

All of models in Digidesign Digi00x family have the same formation of
data channels in isochronous packet for both directions. This commit
simplifies allocation of isochronous resources in this point.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/digi00x/digi00x-stream.c | 42 +++++++++++--------------
 1 file changed, 18 insertions(+), 24 deletions(-)

diff --git a/sound/firewire/digi00x/digi00x-stream.c b/sound/firewire/digi00x/digi00x-stream.c
index 8104af94aed5..2bddeb3e4bf5 100644
--- a/sound/firewire/digi00x/digi00x-stream.c
+++ b/sound/firewire/digi00x/digi00x-stream.c
@@ -196,12 +196,14 @@ static void release_resources(struct snd_dg00x *dg00x)
 	fw_iso_resources_free(&dg00x->rx_resources);
 }
 
-static int keep_resources(struct snd_dg00x *dg00x, unsigned int rate)
+static int keep_resources(struct snd_dg00x *dg00x, struct amdtp_stream *stream,
+			  unsigned int rate)
 {
-	unsigned int i;
+	struct fw_iso_resources *resources;
+	int i;
 	int err;
 
-	/* Check sampling rate. */
+	// Check sampling rate.
 	for (i = 0; i < SND_DG00X_RATE_COUNT; i++) {
 		if (snd_dg00x_stream_rates[i] == rate)
 			break;
@@ -209,31 +211,19 @@ static int keep_resources(struct snd_dg00x *dg00x, unsigned int rate)
 	if (i == SND_DG00X_RATE_COUNT)
 		return -EINVAL;
 
-	/* Keep resources for out-stream. */
-	err = amdtp_dot_set_parameters(&dg00x->rx_stream, rate,
-				       snd_dg00x_stream_pcm_channels[i]);
-	if (err < 0)
-		return err;
-	err = fw_iso_resources_allocate(&dg00x->rx_resources,
-				amdtp_stream_get_max_payload(&dg00x->rx_stream),
-				fw_parent_device(dg00x->unit)->max_speed);
-	if (err < 0)
-		return err;
+	if (stream == &dg00x->tx_stream)
+		resources = &dg00x->tx_resources;
+	else
+		resources = &dg00x->rx_resources;
 
-	/* Keep resources for in-stream. */
-	err = amdtp_dot_set_parameters(&dg00x->tx_stream, rate,
+	err = amdtp_dot_set_parameters(stream, rate,
 				       snd_dg00x_stream_pcm_channels[i]);
 	if (err < 0)
 		return err;
-	err = fw_iso_resources_allocate(&dg00x->tx_resources,
-				amdtp_stream_get_max_payload(&dg00x->tx_stream),
-				fw_parent_device(dg00x->unit)->max_speed);
-	if (err < 0) {
-		fw_iso_resources_free(&dg00x->rx_resources);
-		return err;
-	}
 
-	return 0;
+	return fw_iso_resources_allocate(resources,
+				amdtp_stream_get_max_payload(stream),
+				fw_parent_device(dg00x->unit)->max_speed);
 }
 
 int snd_dg00x_stream_init_duplex(struct snd_dg00x *dg00x)
@@ -306,7 +296,11 @@ int snd_dg00x_stream_start_duplex(struct snd_dg00x *dg00x, unsigned int rate)
 		if (err < 0)
 			goto error;
 
-		err = keep_resources(dg00x, rate);
+		err = keep_resources(dg00x, &dg00x->rx_stream, rate);
+		if (err < 0)
+			goto error;
+
+		err = keep_resources(dg00x, &dg00x->tx_stream, rate);
 		if (err < 0)
 			goto error;
 
-- 
2.20.1

[PATCH 05/12] ALSA: firewire-digi00x: reserve/release isochronous resources in pcm.hw_params/hw_free callbacks

[Takashi Sakamoto]

Once allocated, isochronous resources are available for packet
streaming, even if the streaming is cancelled. For this reason,
current implementation handles allocation of the resources and
starting packet streaming at the same time. However, this brings
complicated procedure to start packet streaming.

This commit separates the allocation and starting. The allocation is
done in pcm.hw_params callback and available till pcm.hw_free callback.
Even if any XRUN occurs, pcm.prepare callback is done to restart
packet streaming without releasing/allocating the resources.

There are two points to stop packet streaming; in pcm.hw_params and
pcm.prepare callbacks. The former point is a case that packet streaming
is already started for any MIDI substream then packet streaming is
requested with different sampling transfer frequency for any PCM
substream. The latter point is cases of any XRUN or packet queueing
error.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/digi00x/digi00x-midi.c   | 10 ++-
 sound/firewire/digi00x/digi00x-pcm.c    | 28 ++++++---
 sound/firewire/digi00x/digi00x-stream.c | 82 ++++++++++++++-----------
 sound/firewire/digi00x/digi00x.h        |  4 +-
 4 files changed, 73 insertions(+), 51 deletions(-)

diff --git a/sound/firewire/digi00x/digi00x-midi.c b/sound/firewire/digi00x/digi00x-midi.c
index 7ab3d0810f6b..cca888cce0d3 100644
--- a/sound/firewire/digi00x/digi00x-midi.c
+++ b/sound/firewire/digi00x/digi00x-midi.c
@@ -18,8 +18,11 @@ static int midi_open(struct snd_rawmidi_substream *substream)
 		return err;
 
 	mutex_lock(&dg00x->mutex);
-	dg00x->substreams_counter++;
-	err = snd_dg00x_stream_start_duplex(dg00x, 0);
+	err = snd_dg00x_stream_reserve_duplex(dg00x, 0);
+	if (err >= 0) {
+		++dg00x->substreams_counter;
+		err = snd_dg00x_stream_start_duplex(dg00x);
+	}
 	mutex_unlock(&dg00x->mutex);
 	if (err < 0)
 		snd_dg00x_stream_lock_release(dg00x);
@@ -32,8 +35,9 @@ static int midi_close(struct snd_rawmidi_substream *substream)
 	struct snd_dg00x *dg00x = substream->rmidi->private_data;
 
 	mutex_lock(&dg00x->mutex);
-	dg00x->substreams_counter--;
+	--dg00x->substreams_counter;
 	snd_dg00x_stream_stop_duplex(dg00x);
+	snd_dg00x_stream_release_duplex(dg00x);
 	mutex_unlock(&dg00x->mutex);
 
 	snd_dg00x_stream_lock_release(dg00x);
diff --git a/sound/firewire/digi00x/digi00x-pcm.c b/sound/firewire/digi00x/digi00x-pcm.c
index fdcff0460c53..1c04747f4fcc 100644
--- a/sound/firewire/digi00x/digi00x-pcm.c
+++ b/sound/firewire/digi00x/digi00x-pcm.c
@@ -167,12 +167,16 @@ static int pcm_capture_hw_params(struct snd_pcm_substream *substream,
 		return err;
 
 	if (substream->runtime->status->state == SNDRV_PCM_STATE_OPEN) {
+		unsigned int rate = params_rate(hw_params);
+
 		mutex_lock(&dg00x->mutex);
-		dg00x->substreams_counter++;
+		err = snd_dg00x_stream_reserve_duplex(dg00x, rate);
+		if (err >= 0)
+			++dg00x->substreams_counter;
 		mutex_unlock(&dg00x->mutex);
 	}
 
-	return 0;
+	return err;
 }
 
 static int pcm_playback_hw_params(struct snd_pcm_substream *substream,
@@ -187,12 +191,16 @@ static int pcm_playback_hw_params(struct snd_pcm_substream *substream,
 		return err;
 
 	if (substream->runtime->status->state == SNDRV_PCM_STATE_OPEN) {
+		unsigned int rate = params_rate(hw_params);
+
 		mutex_lock(&dg00x->mutex);
-		dg00x->substreams_counter++;
+		err = snd_dg00x_stream_reserve_duplex(dg00x, rate);
+		if (err >= 0)
+			++dg00x->substreams_counter;
 		mutex_unlock(&dg00x->mutex);
 	}
 
-	return 0;
+	return err;
 }
 
 static int pcm_capture_hw_free(struct snd_pcm_substream *substream)
@@ -202,9 +210,10 @@ static int pcm_capture_hw_free(struct snd_pcm_substream *substream)
 	mutex_lock(&dg00x->mutex);
 
 	if (substream->runtime->status->state != SNDRV_PCM_STATE_OPEN)
-		dg00x->substreams_counter--;
+		--dg00x->substreams_counter;
 
 	snd_dg00x_stream_stop_duplex(dg00x);
+	snd_dg00x_stream_release_duplex(dg00x);
 
 	mutex_unlock(&dg00x->mutex);
 
@@ -218,9 +227,10 @@ static int pcm_playback_hw_free(struct snd_pcm_substream *substream)
 	mutex_lock(&dg00x->mutex);
 
 	if (substream->runtime->status->state != SNDRV_PCM_STATE_OPEN)
-		dg00x->substreams_counter--;
+		--dg00x->substreams_counter;
 
 	snd_dg00x_stream_stop_duplex(dg00x);
+	snd_dg00x_stream_release_duplex(dg00x);
 
 	mutex_unlock(&dg00x->mutex);
 
@@ -230,12 +240,11 @@ static int pcm_playback_hw_free(struct snd_pcm_substream *substream)
 static int pcm_capture_prepare(struct snd_pcm_substream *substream)
 {
 	struct snd_dg00x *dg00x = substream->private_data;
-	struct snd_pcm_runtime *runtime = substream->runtime;
 	int err;
 
 	mutex_lock(&dg00x->mutex);
 
-	err = snd_dg00x_stream_start_duplex(dg00x, runtime->rate);
+	err = snd_dg00x_stream_start_duplex(dg00x);
 	if (err >= 0)
 		amdtp_stream_pcm_prepare(&dg00x->tx_stream);
 
@@ -247,12 +256,11 @@ static int pcm_capture_prepare(struct snd_pcm_substream *substream)
 static int pcm_playback_prepare(struct snd_pcm_substream *substream)
 {
 	struct snd_dg00x *dg00x = substream->private_data;
-	struct snd_pcm_runtime *runtime = substream->runtime;
 	int err;
 
 	mutex_lock(&dg00x->mutex);
 
-	err = snd_dg00x_stream_start_duplex(dg00x, runtime->rate);
+	err = snd_dg00x_stream_start_duplex(dg00x);
 	if (err >= 0) {
 		amdtp_stream_pcm_prepare(&dg00x->rx_stream);
 		amdtp_dot_reset(&dg00x->rx_stream);
diff --git a/sound/firewire/digi00x/digi00x-stream.c b/sound/firewire/digi00x/digi00x-stream.c
index 2bddeb3e4bf5..3b903e42d29a 100644
--- a/sound/firewire/digi00x/digi00x-stream.c
+++ b/sound/firewire/digi00x/digi00x-stream.c
@@ -189,13 +189,6 @@ static int begin_session(struct snd_dg00x *dg00x)
 	return err;
 }
 
-static void release_resources(struct snd_dg00x *dg00x)
-{
-	/* Release isochronous resources. */
-	fw_iso_resources_free(&dg00x->tx_resources);
-	fw_iso_resources_free(&dg00x->rx_resources);
-}
-
 static int keep_resources(struct snd_dg00x *dg00x, struct amdtp_stream *stream,
 			  unsigned int rate)
 {
@@ -265,45 +258,65 @@ void snd_dg00x_stream_destroy_duplex(struct snd_dg00x *dg00x)
 	fw_iso_resources_destroy(&dg00x->tx_resources);
 }
 
-int snd_dg00x_stream_start_duplex(struct snd_dg00x *dg00x, unsigned int rate)
+int snd_dg00x_stream_reserve_duplex(struct snd_dg00x *dg00x, unsigned int rate)
 {
 	unsigned int curr_rate;
-	int err = 0;
-
-	if (dg00x->substreams_counter == 0)
-		goto end;
+	int err;
 
-	/* Check current sampling rate. */
 	err = snd_dg00x_stream_get_local_rate(dg00x, &curr_rate);
 	if (err < 0)
-		goto error;
+		return err;
 	if (rate == 0)
 		rate = curr_rate;
-	if (curr_rate != rate ||
-	    amdtp_streaming_error(&dg00x->tx_stream) ||
-	    amdtp_streaming_error(&dg00x->rx_stream)) {
+
+	if (dg00x->substreams_counter == 0 || curr_rate != rate) {
 		finish_session(dg00x);
 
-		release_resources(dg00x);
-	}
+		fw_iso_resources_free(&dg00x->tx_resources);
+		fw_iso_resources_free(&dg00x->rx_resources);
 
-	/*
-	 * No packets are transmitted without receiving packets, reagardless of
-	 * which source of clock is used.
-	 */
-	if (!amdtp_stream_running(&dg00x->rx_stream)) {
 		err = snd_dg00x_stream_set_local_rate(dg00x, rate);
 		if (err < 0)
-			goto error;
+			return err;
 
 		err = keep_resources(dg00x, &dg00x->rx_stream, rate);
 		if (err < 0)
-			goto error;
+			return err;
 
 		err = keep_resources(dg00x, &dg00x->tx_stream, rate);
-		if (err < 0)
-			goto error;
+		if (err < 0) {
+			fw_iso_resources_free(&dg00x->rx_resources);
+			return err;
+		}
+	}
+
+	return 0;
+}
 
+void snd_dg00x_stream_release_duplex(struct snd_dg00x *dg00x)
+{
+	if (dg00x->substreams_counter == 0) {
+		fw_iso_resources_free(&dg00x->tx_resources);
+		fw_iso_resources_free(&dg00x->rx_resources);
+	}
+}
+
+int snd_dg00x_stream_start_duplex(struct snd_dg00x *dg00x)
+{
+	int err = 0;
+
+	if (dg00x->substreams_counter == 0)
+		return 0;
+
+	if (amdtp_streaming_error(&dg00x->tx_stream) ||
+	    amdtp_streaming_error(&dg00x->rx_stream))
+		finish_session(dg00x);
+
+	/*
+	 * No packets are transmitted without receiving packets, reagardless of
+	 * which source of clock is used.
+	 */
+	if (!amdtp_stream_running(&dg00x->rx_stream)) {
 		err = begin_session(dg00x);
 		if (err < 0)
 			goto error;
@@ -338,23 +351,18 @@ int snd_dg00x_stream_start_duplex(struct snd_dg00x *dg00x, unsigned int rate)
 			goto error;
 		}
 	}
-end:
-	return err;
+
+	return 0;
 error:
 	finish_session(dg00x);
 
-	release_resources(dg00x);
-
 	return err;
 }
 
 void snd_dg00x_stream_stop_duplex(struct snd_dg00x *dg00x)
 {
-	if (dg00x->substreams_counter > 0)
-		return;
-
-	finish_session(dg00x);
-	release_resources(dg00x);
+	if (dg00x->substreams_counter == 0)
+		finish_session(dg00x);
 }
 
 void snd_dg00x_stream_update_duplex(struct snd_dg00x *dg00x)
diff --git a/sound/firewire/digi00x/digi00x.h b/sound/firewire/digi00x/digi00x.h
index 4dd1bbf2ed3c..3fb1c49f6f9e 100644
--- a/sound/firewire/digi00x/digi00x.h
+++ b/sound/firewire/digi00x/digi00x.h
@@ -140,8 +140,10 @@ int snd_dg00x_stream_get_clock(struct snd_dg00x *dg00x,
 int snd_dg00x_stream_check_external_clock(struct snd_dg00x *dg00x,
 					  bool *detect);
 int snd_dg00x_stream_init_duplex(struct snd_dg00x *dg00x);
-int snd_dg00x_stream_start_duplex(struct snd_dg00x *dg00x, unsigned int rate);
+int snd_dg00x_stream_reserve_duplex(struct snd_dg00x *dg00x, unsigned int rate);
+int snd_dg00x_stream_start_duplex(struct snd_dg00x *dg00x);
 void snd_dg00x_stream_stop_duplex(struct snd_dg00x *dg00x);
+void snd_dg00x_stream_release_duplex(struct snd_dg00x *dg00x);
 void snd_dg00x_stream_update_duplex(struct snd_dg00x *dg00x);
 void snd_dg00x_stream_destroy_duplex(struct snd_dg00x *dg00x);
 
-- 
2.20.1

[PATCH 06/12] ALSA: firewire-digi00x: update isochronous resources when starting packet streaming after bus-reset

[Takashi Sakamoto]

After bus reset, isochronous resource manager releases all of allocated
isochronous resources. The nodes to transfer isochronous packet should
request reallocation of the resources.

However, between the bus-reset and invocation of 'struct fw_driver.update'
handler, ALSA PCM application can detect this situation by XRUN because
the target device cancelled to transmit packets once bus-reset occurs.

Due to the above mechanism, ALSA fireface driver just stops packet
streaming in the update handler, thus pcm.prepare handler should
request the reallocation.

This commit requests the reallocation in pcm.prepare callback when
bus generation is changed.

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

diff --git a/sound/firewire/digi00x/digi00x-stream.c b/sound/firewire/digi00x/digi00x-stream.c
index 3b903e42d29a..3c5e1c5a2e11 100644
--- a/sound/firewire/digi00x/digi00x-stream.c
+++ b/sound/firewire/digi00x/digi00x-stream.c
@@ -303,6 +303,7 @@ void snd_dg00x_stream_release_duplex(struct snd_dg00x *dg00x)
 
 int snd_dg00x_stream_start_duplex(struct snd_dg00x *dg00x)
 {
+	unsigned int generation = dg00x->rx_resources.generation;
 	int err = 0;
 
 	if (dg00x->substreams_counter == 0)
@@ -312,6 +313,16 @@ int snd_dg00x_stream_start_duplex(struct snd_dg00x *dg00x)
 	    amdtp_streaming_error(&dg00x->rx_stream))
 		finish_session(dg00x);
 
+	if (generation != fw_parent_device(dg00x->unit)->card->generation) {
+		err = fw_iso_resources_update(&dg00x->tx_resources);
+		if (err < 0)
+			goto error;
+
+		err = fw_iso_resources_update(&dg00x->rx_resources);
+		if (err < 0)
+			goto error;
+	}
+
 	/*
 	 * No packets are transmitted without receiving packets, reagardless of
 	 * which source of clock is used.
-- 
2.20.1

[PATCH 07/12] ALSA: firewire-digi00x: code refactoring for pcm.hw_params/hw_free callbacks

[Takashi Sakamoto]

The pairs of pcm.hw_params callbacks and .hw_free callbacks for both
direction have no differences.

This commit unifies the pairs.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/digi00x/digi00x-pcm.c | 55 ++++------------------------
 1 file changed, 7 insertions(+), 48 deletions(-)

diff --git a/sound/firewire/digi00x/digi00x-pcm.c b/sound/firewire/digi00x/digi00x-pcm.c
index 1c04747f4fcc..9ed2ebdcf23a 100644
--- a/sound/firewire/digi00x/digi00x-pcm.c
+++ b/sound/firewire/digi00x/digi00x-pcm.c
@@ -155,8 +155,8 @@ static int pcm_close(struct snd_pcm_substream *substream)
 	return 0;
 }
 
-static int pcm_capture_hw_params(struct snd_pcm_substream *substream,
-				 struct snd_pcm_hw_params *hw_params)
+static int pcm_hw_params(struct snd_pcm_substream *substream,
+			 struct snd_pcm_hw_params *hw_params)
 {
 	struct snd_dg00x *dg00x = substream->private_data;
 	int err;
@@ -179,48 +179,7 @@ static int pcm_capture_hw_params(struct snd_pcm_substream *substream,
 	return err;
 }
 
-static int pcm_playback_hw_params(struct snd_pcm_substream *substream,
-				  struct snd_pcm_hw_params *hw_params)
-{
-	struct snd_dg00x *dg00x = substream->private_data;
-	int err;
-
-	err = snd_pcm_lib_alloc_vmalloc_buffer(substream,
-					       params_buffer_bytes(hw_params));
-	if (err < 0)
-		return err;
-
-	if (substream->runtime->status->state == SNDRV_PCM_STATE_OPEN) {
-		unsigned int rate = params_rate(hw_params);
-
-		mutex_lock(&dg00x->mutex);
-		err = snd_dg00x_stream_reserve_duplex(dg00x, rate);
-		if (err >= 0)
-			++dg00x->substreams_counter;
-		mutex_unlock(&dg00x->mutex);
-	}
-
-	return err;
-}
-
-static int pcm_capture_hw_free(struct snd_pcm_substream *substream)
-{
-	struct snd_dg00x *dg00x = substream->private_data;
-
-	mutex_lock(&dg00x->mutex);
-
-	if (substream->runtime->status->state != SNDRV_PCM_STATE_OPEN)
-		--dg00x->substreams_counter;
-
-	snd_dg00x_stream_stop_duplex(dg00x);
-	snd_dg00x_stream_release_duplex(dg00x);
-
-	mutex_unlock(&dg00x->mutex);
-
-	return snd_pcm_lib_free_vmalloc_buffer(substream);
-}
-
-static int pcm_playback_hw_free(struct snd_pcm_substream *substream)
+static int pcm_hw_free(struct snd_pcm_substream *substream)
 {
 	struct snd_dg00x *dg00x = substream->private_data;
 
@@ -341,8 +300,8 @@ int snd_dg00x_create_pcm_devices(struct snd_dg00x *dg00x)
 		.open		= pcm_open,
 		.close		= pcm_close,
 		.ioctl		= snd_pcm_lib_ioctl,
-		.hw_params	= pcm_capture_hw_params,
-		.hw_free	= pcm_capture_hw_free,
+		.hw_params	= pcm_hw_params,
+		.hw_free	= pcm_hw_free,
 		.prepare	= pcm_capture_prepare,
 		.trigger	= pcm_capture_trigger,
 		.pointer	= pcm_capture_pointer,
@@ -353,8 +312,8 @@ int snd_dg00x_create_pcm_devices(struct snd_dg00x *dg00x)
 		.open		= pcm_open,
 		.close		= pcm_close,
 		.ioctl		= snd_pcm_lib_ioctl,
-		.hw_params	= pcm_playback_hw_params,
-		.hw_free	= pcm_playback_hw_free,
+		.hw_params	= pcm_hw_params,
+		.hw_free	= pcm_hw_free,
 		.prepare	= pcm_playback_prepare,
 		.trigger	= pcm_playback_trigger,
 		.pointer	= pcm_playback_pointer,
-- 
2.20.1

[PATCH 08/12] ALSA: dice: code refactoring to stop packet streaming

[Takashi Sakamoto]

This commit is a part of preparation to perform allocation/release
of isochronous resources in pcm.hw_params/hw_free callbacks.

There're three points to finish packet streaming but no helper
functions for common operations for it. This commit adds a helper
function for operations to finish packet streaming.

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

diff --git a/sound/firewire/dice/dice-stream.c b/sound/firewire/dice/dice-stream.c
index c3c892c5c7ff..8bce923dc4bd 100644
--- a/sound/firewire/dice/dice-stream.c
+++ b/sound/firewire/dice/dice-stream.c
@@ -230,6 +230,15 @@ static int keep_resources(struct snd_dice *dice,
 				fw_parent_device(dice->unit)->max_speed);
 }
 
+static void finish_session(struct snd_dice *dice, struct reg_params *tx_params,
+			   struct reg_params *rx_params)
+{
+	stop_streams(dice, AMDTP_IN_STREAM, tx_params);
+	stop_streams(dice, AMDTP_OUT_STREAM, rx_params);
+
+	snd_dice_transaction_clear_enable(dice);
+}
+
 static int start_streams(struct snd_dice *dice, enum amdtp_stream_direction dir,
 			 unsigned int rate, struct reg_params *params)
 {
@@ -328,10 +337,8 @@ static int start_duplex_streams(struct snd_dice *dice, unsigned int rate)
 	if (err < 0)
 		return err;
 
-	/* Stop transmission. */
-	stop_streams(dice, AMDTP_IN_STREAM, &tx_params);
-	stop_streams(dice, AMDTP_OUT_STREAM, &rx_params);
-	snd_dice_transaction_clear_enable(dice);
+	// Stop transmission.
+	finish_session(dice, &tx_params, &rx_params);
 	release_resources(dice);
 
 	err = ensure_phase_lock(dice, rate);
@@ -373,9 +380,7 @@ static int start_duplex_streams(struct snd_dice *dice, unsigned int rate)
 
 	return 0;
 error:
-	stop_streams(dice, AMDTP_IN_STREAM, &tx_params);
-	stop_streams(dice, AMDTP_OUT_STREAM, &rx_params);
-	snd_dice_transaction_clear_enable(dice);
+	finish_session(dice, &tx_params, &rx_params);
 	release_resources(dice);
 	return err;
 }
@@ -449,12 +454,8 @@ void snd_dice_stream_stop_duplex(struct snd_dice *dice)
 	if (dice->substreams_counter > 0)
 		return;
 
-	snd_dice_transaction_clear_enable(dice);
-
-	if (get_register_params(dice, &tx_params, &rx_params) == 0) {
-		stop_streams(dice, AMDTP_IN_STREAM, &tx_params);
-		stop_streams(dice, AMDTP_OUT_STREAM, &rx_params);
-	}
+	if (get_register_params(dice, &tx_params, &rx_params) >= 0)
+		finish_session(dice, &tx_params, &rx_params);
 
 	release_resources(dice);
 }
-- 
2.20.1

[PATCH 09/12] ALSA: dice: code refactoring to keep isochronous resources

[Takashi Sakamoto]

This commit is a part of preparation to perform allocation/release
of isochronous resources in pcm.hw_params/hw_free callbacks.

This commit adds a helper function to allocate isochronous resources,
separated from operations to start packet streaming, I note that some
dice-based devices have two pair of endpoints for isochronous packet
straeming.

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

diff --git a/sound/firewire/dice/dice-stream.c b/sound/firewire/dice/dice-stream.c
index 8bce923dc4bd..010cbf02de4f 100644
--- a/sound/firewire/dice/dice-stream.c
+++ b/sound/firewire/dice/dice-stream.c
@@ -175,35 +175,22 @@ static void stop_streams(struct snd_dice *dice, enum amdtp_stream_direction dir,
 	}
 }
 
-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)
+static int keep_resources(struct snd_dice *dice, struct amdtp_stream *stream,
+			  struct fw_iso_resources *resources, unsigned int rate,
+			  unsigned int pcm_chs, unsigned int midi_ports)
 {
-	struct amdtp_stream *stream;
-	struct fw_iso_resources *resources;
 	bool double_pcm_frames;
 	unsigned int i;
 	int err;
 
-	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];
-	}
-
-	/*
-	 * 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
-	 * a half of PCM sampling frequency, i.e. PCM frames at 192.0 kHz are
-	 * transferred on AMDTP packets at 96 kHz. Two successive samples of a
-	 * channel are stored consecutively in the packet. This quirk is called
-	 * as 'Dual Wire'.
-	 * For this quirk, blocking mode is required and PCM buffer size should
-	 * be aligned to SYT_INTERVAL.
-	 */
+	// 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
+	// a half of PCM sampling frequency, i.e. PCM frames at 192.0 kHz are
+	// transferred on AMDTP packets at 96 kHz. Two successive samples of a
+	// channel are stored consecutively in the packet. This quirk is called
+	// as 'Dual Wire'.
+	// For this quirk, blocking mode is required and PCM buffer size should
+	// be aligned to SYT_INTERVAL.
 	double_pcm_frames = rate > 96000;
 	if (double_pcm_frames) {
 		rate /= 2;
@@ -230,49 +217,40 @@ static int keep_resources(struct snd_dice *dice,
 				fw_parent_device(dice->unit)->max_speed);
 }
 
-static void finish_session(struct snd_dice *dice, struct reg_params *tx_params,
-			   struct reg_params *rx_params)
+static int keep_dual_resources(struct snd_dice *dice, unsigned int rate,
+			       enum amdtp_stream_direction dir,
+			       struct reg_params *params)
 {
-	stop_streams(dice, AMDTP_IN_STREAM, tx_params);
-	stop_streams(dice, AMDTP_OUT_STREAM, rx_params);
-
-	snd_dice_transaction_clear_enable(dice);
-}
-
-static int start_streams(struct snd_dice *dice, enum amdtp_stream_direction dir,
-			 unsigned int rate, struct reg_params *params)
-{
-	__be32 reg[2];
 	enum snd_dice_rate_mode mode;
-	unsigned int i, pcm_chs, midi_ports;
-	struct amdtp_stream *streams;
-	struct fw_iso_resources *resources;
-	struct fw_device *fw_dev = fw_parent_device(dice->unit);
-	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;
-	}
+	int i;
+	int err;
 
 	err = snd_dice_stream_get_rate_mode(dice, rate, &mode);
 	if (err < 0)
 		return err;
 
-	for (i = 0; i < params->count; i++) {
+	for (i = 0; i < params->count; ++i) {
+		__be32 reg[2];
+		struct amdtp_stream *stream;
+		struct fw_iso_resources *resources;
 		unsigned int pcm_cache;
 		unsigned int midi_cache;
+		unsigned int pcm_chs;
+		unsigned int midi_ports;
 
 		if (dir == AMDTP_IN_STREAM) {
+			stream = &dice->tx_stream[i];
+			resources = &dice->tx_resources[i];
+
 			pcm_cache = dice->tx_pcm_chs[i][mode];
 			midi_cache = dice->tx_midi_ports[i];
 			err = snd_dice_transaction_read_tx(dice,
 					params->size * i + TX_NUMBER_AUDIO,
 					reg, sizeof(reg));
 		} else {
+			stream = &dice->rx_stream[i];
+			resources = &dice->rx_resources[i];
+
 			pcm_cache = dice->rx_pcm_chs[i][mode];
 			midi_cache = dice->rx_midi_ports[i];
 			err = snd_dice_transaction_read_rx(dice,
@@ -284,7 +262,7 @@ static int start_streams(struct snd_dice *dice, enum amdtp_stream_direction dir,
 		pcm_chs = be32_to_cpu(reg[0]);
 		midi_ports = be32_to_cpu(reg[1]);
 
-		/* These are important for developer of this driver. */
+		// These are important for developer of this driver.
 		if (pcm_chs != pcm_cache || midi_ports != midi_cache) {
 			dev_info(&dice->unit->device,
 				 "cache mismatch: pcm: %u:%u, midi: %u:%u\n",
@@ -292,34 +270,71 @@ static int start_streams(struct snd_dice *dice, enum amdtp_stream_direction dir,
 			return -EPROTO;
 		}
 
-		err = keep_resources(dice, dir, i, rate, pcm_chs, midi_ports);
+		err = keep_resources(dice, stream, resources, rate, pcm_chs,
+				     midi_ports);
 		if (err < 0)
 			return err;
+	}
+
+	return 0;
+}
+
+static void finish_session(struct snd_dice *dice, struct reg_params *tx_params,
+			   struct reg_params *rx_params)
+{
+	stop_streams(dice, AMDTP_IN_STREAM, tx_params);
+	stop_streams(dice, AMDTP_OUT_STREAM, rx_params);
+
+	snd_dice_transaction_clear_enable(dice);
+}
+
+static int start_streams(struct snd_dice *dice, enum amdtp_stream_direction dir,
+			 unsigned int rate, struct reg_params *params)
+{
+	unsigned int max_speed = fw_parent_device(dice->unit)->max_speed;
+	int i;
+	int err;
+
+	err = keep_dual_resources(dice, rate, dir, params);
+	if (err < 0)
+		return err;
+
+	for (i = 0; i < params->count; i++) {
+		struct amdtp_stream *stream;
+		struct fw_iso_resources *resources;
+		__be32 reg;
+
+		if (dir == AMDTP_IN_STREAM) {
+			stream = dice->tx_stream + i;
+			resources = dice->tx_resources + i;
+		} else {
+			stream = dice->rx_stream + i;
+			resources = dice->rx_resources + i;
+		}
 
-		reg[0] = cpu_to_be32(resources[i].channel);
+		reg = cpu_to_be32(resources->channel);
 		if (dir == AMDTP_IN_STREAM) {
 			err = snd_dice_transaction_write_tx(dice,
 					params->size * i + TX_ISOCHRONOUS,
-					reg, sizeof(reg[0]));
+					&reg, sizeof(reg));
 		} else {
 			err = snd_dice_transaction_write_rx(dice,
 					params->size * i + RX_ISOCHRONOUS,
-					reg, sizeof(reg[0]));
+					&reg, sizeof(reg));
 		}
 		if (err < 0)
 			return err;
 
 		if (dir == AMDTP_IN_STREAM) {
-			reg[0] = cpu_to_be32(fw_dev->max_speed);
+			reg = cpu_to_be32(max_speed);
 			err = snd_dice_transaction_write_tx(dice,
 					params->size * i + TX_SPEED,
-					reg, sizeof(reg[0]));
+					&reg, sizeof(reg));
 			if (err < 0)
 				return err;
 		}
 
-		err = amdtp_stream_start(&streams[i], resources[i].channel,
-					 fw_dev->max_speed);
+		err = amdtp_stream_start(stream, resources->channel, max_speed);
 		if (err < 0)
 			return err;
 	}
-- 
2.20.1

[PATCH 10/12] ALSA: dice: reserve/release isochronous resources in pcm.hw_params/hw_free callbacks

[Takashi Sakamoto]

Once allocated, isochronous resources are available for packet
streaming, even if the streaming is cancelled. For this reason,
current implementation handles allocation of the resources and
starting packet streaming at the same time. However, this brings
complicated procedure to start packet streaming.

This commit separates the allocation and starting. The allocation is
done in pcm.hw_params callback and available till pcm.hw_free callback.
Even if any XRUN occurs, pcm.prepare callback is done to restart
packet streaming without releasing/allocating the resources.

There are two points to stop packet streaming; in pcm.hw_params and
pcm.prepare callbacks. The former point is a case that packet streaming
is already started for any MIDI substream then packet streaming is
requested with different sampling transfer frequency for any PCM
substream. The latter point is cases of any XRUN or packet queueing
error.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/dice/dice-midi.c   |  10 +-
 sound/firewire/dice/dice-pcm.c    |  26 ++--
 sound/firewire/dice/dice-stream.c | 218 ++++++++++++++++--------------
 sound/firewire/dice/dice.h        |   4 +-
 4 files changed, 143 insertions(+), 115 deletions(-)

diff --git a/sound/firewire/dice/dice-midi.c b/sound/firewire/dice/dice-midi.c
index 84eca8a51a02..6172dad87c4e 100644
--- a/sound/firewire/dice/dice-midi.c
+++ b/sound/firewire/dice/dice-midi.c
@@ -18,8 +18,11 @@ static int midi_open(struct snd_rawmidi_substream *substream)
 
 	mutex_lock(&dice->mutex);
 
-	dice->substreams_counter++;
-	err = snd_dice_stream_start_duplex(dice, 0);
+	err = snd_dice_stream_reserve_duplex(dice, 0);
+	if (err >= 0) {
+		++dice->substreams_counter;
+		err = snd_dice_stream_start_duplex(dice);
+	}
 
 	mutex_unlock(&dice->mutex);
 
@@ -35,8 +38,9 @@ static int midi_close(struct snd_rawmidi_substream *substream)
 
 	mutex_lock(&dice->mutex);
 
-	dice->substreams_counter--;
+	--dice->substreams_counter;
 	snd_dice_stream_stop_duplex(dice);
+	snd_dice_stream_release_duplex(dice);
 
 	mutex_unlock(&dice->mutex);
 
diff --git a/sound/firewire/dice/dice-pcm.c b/sound/firewire/dice/dice-pcm.c
index bb3ef5ff3488..6c7a6b7ed743 100644
--- a/sound/firewire/dice/dice-pcm.c
+++ b/sound/firewire/dice/dice-pcm.c
@@ -243,12 +243,16 @@ static int capture_hw_params(struct snd_pcm_substream *substream,
 		return err;
 
 	if (substream->runtime->status->state == SNDRV_PCM_STATE_OPEN) {
+		unsigned int rate = params_rate(hw_params);
+
 		mutex_lock(&dice->mutex);
-		dice->substreams_counter++;
+		err = snd_dice_stream_reserve_duplex(dice, rate);
+		if (err >= 0)
+			++dice->substreams_counter;
 		mutex_unlock(&dice->mutex);
 	}
 
-	return 0;
+	return err;
 }
 static int playback_hw_params(struct snd_pcm_substream *substream,
 			      struct snd_pcm_hw_params *hw_params)
@@ -262,12 +266,16 @@ static int playback_hw_params(struct snd_pcm_substream *substream,
 		return err;
 
 	if (substream->runtime->status->state == SNDRV_PCM_STATE_OPEN) {
+		unsigned int rate = params_rate(hw_params);
+
 		mutex_lock(&dice->mutex);
-		dice->substreams_counter++;
+		err = snd_dice_stream_reserve_duplex(dice, rate);
+		if (err >= 0)
+			++dice->substreams_counter;
 		mutex_unlock(&dice->mutex);
 	}
 
-	return 0;
+	return err;
 }
 
 static int capture_hw_free(struct snd_pcm_substream *substream)
@@ -277,9 +285,10 @@ static int capture_hw_free(struct snd_pcm_substream *substream)
 	mutex_lock(&dice->mutex);
 
 	if (substream->runtime->status->state != SNDRV_PCM_STATE_OPEN)
-		dice->substreams_counter--;
+		--dice->substreams_counter;
 
 	snd_dice_stream_stop_duplex(dice);
+	snd_dice_stream_release_duplex(dice);
 
 	mutex_unlock(&dice->mutex);
 
@@ -293,9 +302,10 @@ static int playback_hw_free(struct snd_pcm_substream *substream)
 	mutex_lock(&dice->mutex);
 
 	if (substream->runtime->status->state != SNDRV_PCM_STATE_OPEN)
-		dice->substreams_counter--;
+		--dice->substreams_counter;
 
 	snd_dice_stream_stop_duplex(dice);
+	snd_dice_stream_release_duplex(dice);
 
 	mutex_unlock(&dice->mutex);
 
@@ -309,7 +319,7 @@ static int capture_prepare(struct snd_pcm_substream *substream)
 	int err;
 
 	mutex_lock(&dice->mutex);
-	err = snd_dice_stream_start_duplex(dice, substream->runtime->rate);
+	err = snd_dice_stream_start_duplex(dice);
 	mutex_unlock(&dice->mutex);
 	if (err >= 0)
 		amdtp_stream_pcm_prepare(stream);
@@ -323,7 +333,7 @@ static int playback_prepare(struct snd_pcm_substream *substream)
 	int err;
 
 	mutex_lock(&dice->mutex);
-	err = snd_dice_stream_start_duplex(dice, substream->runtime->rate);
+	err = snd_dice_stream_start_duplex(dice);
 	mutex_unlock(&dice->mutex);
 	if (err >= 0)
 		amdtp_stream_pcm_prepare(stream);
diff --git a/sound/firewire/dice/dice-stream.c b/sound/firewire/dice/dice-stream.c
index 010cbf02de4f..6bbf7421a53c 100644
--- a/sound/firewire/dice/dice-stream.c
+++ b/sound/firewire/dice/dice-stream.c
@@ -138,18 +138,9 @@ static int get_register_params(struct snd_dice *dice,
 
 static void release_resources(struct snd_dice *dice)
 {
-	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]);
-		}
+	int i;
 
+	for (i = 0; i < MAX_STREAMS; ++i) {
 		fw_iso_resources_free(&dice->tx_resources[i]);
 		fw_iso_resources_free(&dice->rx_resources[i]);
 	}
@@ -164,10 +155,14 @@ static void stop_streams(struct snd_dice *dice, enum amdtp_stream_direction dir,
 	for (i = 0; i < params->count; i++) {
 		reg = cpu_to_be32((u32)-1);
 		if (dir == AMDTP_IN_STREAM) {
+			amdtp_stream_stop(&dice->tx_stream[i]);
+
 			snd_dice_transaction_write_tx(dice,
 					params->size * i + TX_ISOCHRONOUS,
 					&reg, sizeof(reg));
 		} else {
+			amdtp_stream_stop(&dice->rx_stream[i]);
+
 			snd_dice_transaction_write_rx(dice,
 					params->size * i + RX_ISOCHRONOUS,
 					&reg, sizeof(reg));
@@ -288,6 +283,65 @@ static void finish_session(struct snd_dice *dice, struct reg_params *tx_params,
 	snd_dice_transaction_clear_enable(dice);
 }
 
+int snd_dice_stream_reserve_duplex(struct snd_dice *dice, unsigned int rate)
+{
+	unsigned int curr_rate;
+	int err;
+
+	// Check sampling transmission frequency.
+	err = snd_dice_transaction_get_rate(dice, &curr_rate);
+	if (err < 0)
+		return err;
+	if (rate == 0)
+		rate = curr_rate;
+
+	if (dice->substreams_counter == 0 || curr_rate != rate) {
+		struct reg_params tx_params, rx_params;
+
+		err = get_register_params(dice, &tx_params, &rx_params);
+		if (err < 0)
+			return err;
+
+		finish_session(dice, &tx_params, &rx_params);
+
+		release_resources(dice);
+
+		// Just after owning the unit (GLOBAL_OWNER), the unit can
+		// return invalid stream formats. Selecting clock parameters
+		// have an effect for the unit to refine it.
+		err = ensure_phase_lock(dice, rate);
+		if (err < 0)
+			return err;
+
+		// After changing sampling transfer frequency, the value of
+		// register can be changed.
+		err = get_register_params(dice, &tx_params, &rx_params);
+		if (err < 0)
+			return err;
+
+		err = keep_dual_resources(dice, rate, AMDTP_IN_STREAM,
+					  &tx_params);
+		if (err < 0)
+			goto error;
+
+		err = keep_dual_resources(dice, rate, AMDTP_OUT_STREAM,
+					  &rx_params);
+		if (err < 0)
+			goto error;
+	}
+
+	return 0;
+error:
+	release_resources(dice);
+	return err;
+}
+
+void snd_dice_stream_release_duplex(struct snd_dice *dice)
+{
+	if (dice->substreams_counter == 0)
+		release_resources(dice);
+}
+
 static int start_streams(struct snd_dice *dice, enum amdtp_stream_direction dir,
 			 unsigned int rate, struct reg_params *params)
 {
@@ -295,10 +349,6 @@ static int start_streams(struct snd_dice *dice, enum amdtp_stream_direction dir,
 	int i;
 	int err;
 
-	err = keep_dual_resources(dice, rate, dir, params);
-	if (err < 0)
-		return err;
-
 	for (i = 0; i < params->count; i++) {
 		struct amdtp_stream *stream;
 		struct fw_iso_resources *resources;
@@ -342,102 +392,39 @@ static int start_streams(struct snd_dice *dice, enum amdtp_stream_direction dir,
 	return err;
 }
 
-static int start_duplex_streams(struct snd_dice *dice, unsigned int rate)
-{
-	struct reg_params tx_params, rx_params;
-	int i;
-	int err;
-
-	err = get_register_params(dice, &tx_params, &rx_params);
-	if (err < 0)
-		return err;
-
-	// Stop transmission.
-	finish_session(dice, &tx_params, &rx_params);
-	release_resources(dice);
-
-	err = ensure_phase_lock(dice, rate);
-	if (err < 0) {
-		dev_err(&dice->unit->device, "fail to ensure phase lock\n");
-		return err;
-	}
-
-	/* Likely to have changed stream formats. */
-	err = get_register_params(dice, &tx_params, &rx_params);
-	if (err < 0)
-		return err;
-
-	/* Start both streams. */
-	err = start_streams(dice, AMDTP_IN_STREAM, rate, &tx_params);
-	if (err < 0)
-		goto error;
-	err = start_streams(dice, AMDTP_OUT_STREAM, rate, &rx_params);
-	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");
-		goto error;
-	}
-
-	for (i = 0; i < MAX_STREAMS; i++) {
-		if ((i < tx_params.count &&
-		    !amdtp_stream_wait_callback(&dice->tx_stream[i],
-						CALLBACK_TIMEOUT)) ||
-		    (i < rx_params.count &&
-		     !amdtp_stream_wait_callback(&dice->rx_stream[i],
-						 CALLBACK_TIMEOUT))) {
-			err = -ETIMEDOUT;
-			goto error;
-		}
-	}
-
-	return 0;
-error:
-	finish_session(dice, &tx_params, &rx_params);
-	release_resources(dice);
-	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)
+int snd_dice_stream_start_duplex(struct snd_dice *dice)
 {
-	unsigned int curr_rate;
+	struct reg_params tx_params, rx_params;
 	unsigned int i;
+	unsigned int rate;
 	enum snd_dice_rate_mode mode;
 	int err;
 
 	if (dice->substreams_counter == 0)
 		return -EIO;
 
-	/* Check sampling transmission frequency. */
-	err = snd_dice_transaction_get_rate(dice, &curr_rate);
-	if (err < 0) {
-		dev_err(&dice->unit->device,
-			"fail to get sampling rate\n");
+	err = get_register_params(dice, &tx_params, &rx_params);
+	if (err < 0)
 		return err;
-	}
-	if (rate == 0)
-		rate = curr_rate;
-	if (rate != curr_rate)
-		goto restart;
 
-	/* Check error of packet streaming. */
+	// Check error of packet streaming.
 	for (i = 0; i < MAX_STREAMS; ++i) {
-		if (amdtp_streaming_error(&dice->tx_stream[i]))
-			break;
-		if (amdtp_streaming_error(&dice->rx_stream[i]))
+		if (amdtp_streaming_error(&dice->tx_stream[i]) ||
+		    amdtp_streaming_error(&dice->rx_stream[i])) {
+			finish_session(dice, &tx_params, &rx_params);
 			break;
+		}
 	}
-	if (i < MAX_STREAMS)
-		goto restart;
 
-	/* Check required streams are running or not. */
+	// Check required streams are running or not.
+	err = snd_dice_transaction_get_rate(dice, &rate);
+	if (err < 0)
+		return err;
 	err = snd_dice_stream_get_rate_mode(dice, rate, &mode);
 	if (err < 0)
 		return err;
@@ -449,12 +436,40 @@ int snd_dice_stream_start_duplex(struct snd_dice *dice, unsigned int rate)
 		    !amdtp_stream_running(&dice->rx_stream[i]))
 			break;
 	}
-	if (i < MAX_STREAMS)
-		goto restart;
+	if (i < MAX_STREAMS) {
+		// Start both streams.
+		err = start_streams(dice, AMDTP_IN_STREAM, rate, &tx_params);
+		if (err < 0)
+			goto error;
+
+		err = start_streams(dice, AMDTP_OUT_STREAM, rate, &rx_params);
+		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");
+			goto error;
+		}
+
+		for (i = 0; i < MAX_STREAMS; i++) {
+			if ((i < tx_params.count &&
+			    !amdtp_stream_wait_callback(&dice->tx_stream[i],
+							CALLBACK_TIMEOUT)) ||
+			    (i < rx_params.count &&
+			     !amdtp_stream_wait_callback(&dice->rx_stream[i],
+							 CALLBACK_TIMEOUT))) {
+				err = -ETIMEDOUT;
+				goto error;
+			}
+		}
+	}
 
 	return 0;
-restart:
-	return start_duplex_streams(dice, rate);
+error:
+	finish_session(dice, &tx_params, &rx_params);
+	return err;
 }
 
 /*
@@ -466,13 +481,10 @@ void snd_dice_stream_stop_duplex(struct snd_dice *dice)
 {
 	struct reg_params tx_params, rx_params;
 
-	if (dice->substreams_counter > 0)
-		return;
-
-	if (get_register_params(dice, &tx_params, &rx_params) >= 0)
-		finish_session(dice, &tx_params, &rx_params);
-
-	release_resources(dice);
+	if (dice->substreams_counter == 0) {
+		if (get_register_params(dice, &tx_params, &rx_params) >= 0)
+			finish_session(dice, &tx_params, &rx_params);
+	}
 }
 
 static int init_stream(struct snd_dice *dice, enum amdtp_stream_direction dir,
diff --git a/sound/firewire/dice/dice.h b/sound/firewire/dice/dice.h
index 9699adc2a96d..f95073b85010 100644
--- a/sound/firewire/dice/dice.h
+++ b/sound/firewire/dice/dice.h
@@ -205,10 +205,12 @@ extern const unsigned int snd_dice_rates[SND_DICE_RATES_COUNT];
 
 int snd_dice_stream_get_rate_mode(struct snd_dice *dice, unsigned int rate,
 				  enum snd_dice_rate_mode *mode);
-int snd_dice_stream_start_duplex(struct snd_dice *dice, unsigned int rate);
+int snd_dice_stream_start_duplex(struct snd_dice *dice);
 void snd_dice_stream_stop_duplex(struct snd_dice *dice);
 int snd_dice_stream_init_duplex(struct snd_dice *dice);
 void snd_dice_stream_destroy_duplex(struct snd_dice *dice);
+int snd_dice_stream_reserve_duplex(struct snd_dice *dice, unsigned int rate);
+void snd_dice_stream_release_duplex(struct snd_dice *dice);
 void snd_dice_stream_update_duplex(struct snd_dice *dice);
 int snd_dice_stream_detect_current_formats(struct snd_dice *dice);
 
-- 
2.20.1

[PATCH 11/12] ALSA: dice: update isochronous resources when starting packet streaming after bus-reset

[Takashi Sakamoto]

After bus reset, isochronous resource manager releases all of allocated
isochronous resources. The nodes to transfer isochronous packet should
request reallocation of the resources.

However, between the bus-reset and invocation of 'struct fw_driver.update'
handler, ALSA PCM application can detect this situation by XRUN because
the target device cancelled to transmit packets once bus-reset occurs.

Due to the above mechanism, ALSA fireface driver just stops packet
streaming in the update handler, thus pcm.prepare handler should
request the reallocation.

This commit requests the reallocation in pcm.prepare callback when
bus generation is changed.

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

diff --git a/sound/firewire/dice/dice-stream.c b/sound/firewire/dice/dice-stream.c
index 6bbf7421a53c..f61b99a72655 100644
--- a/sound/firewire/dice/dice-stream.c
+++ b/sound/firewire/dice/dice-stream.c
@@ -399,6 +399,7 @@ static int start_streams(struct snd_dice *dice, enum amdtp_stream_direction dir,
  */
 int snd_dice_stream_start_duplex(struct snd_dice *dice)
 {
+	unsigned int generation = dice->rx_resources[0].generation;
 	struct reg_params tx_params, rx_params;
 	unsigned int i;
 	unsigned int rate;
@@ -421,6 +422,15 @@ int snd_dice_stream_start_duplex(struct snd_dice *dice)
 		}
 	}
 
+	if (generation != fw_parent_device(dice->unit)->card->generation) {
+		for (i = 0; i < MAX_STREAMS; ++i) {
+			if (i < tx_params.count)
+				fw_iso_resources_update(dice->tx_resources + i);
+			if (i < rx_params.count)
+				fw_iso_resources_update(dice->rx_resources + i);
+		}
+	}
+
 	// Check required streams are running or not.
 	err = snd_dice_transaction_get_rate(dice, &rate);
 	if (err < 0)
-- 
2.20.1

[PATCH 12/12] ALSA: dice: code refactoring for pcm.hw_params/hw_free callbacks

[Takashi Sakamoto]

The pairs of pcm.hw_params callbacks and .hw_free callbacks for both
direction have no differences.

This commit unifies the pairs.

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

diff --git a/sound/firewire/dice/dice-pcm.c b/sound/firewire/dice/dice-pcm.c
index 6c7a6b7ed743..00b55dfc3b2c 100644
--- a/sound/firewire/dice/dice-pcm.c
+++ b/sound/firewire/dice/dice-pcm.c
@@ -231,8 +231,8 @@ static int pcm_close(struct snd_pcm_substream *substream)
 	return 0;
 }
 
-static int capture_hw_params(struct snd_pcm_substream *substream,
-			     struct snd_pcm_hw_params *hw_params)
+static int pcm_hw_params(struct snd_pcm_substream *substream,
+			 struct snd_pcm_hw_params *hw_params)
 {
 	struct snd_dice *dice = substream->private_data;
 	int err;
@@ -254,48 +254,8 @@ static int capture_hw_params(struct snd_pcm_substream *substream,
 
 	return err;
 }
-static int playback_hw_params(struct snd_pcm_substream *substream,
-			      struct snd_pcm_hw_params *hw_params)
-{
-	struct snd_dice *dice = substream->private_data;
-	int err;
-
-	err = snd_pcm_lib_alloc_vmalloc_buffer(substream,
-					       params_buffer_bytes(hw_params));
-	if (err < 0)
-		return err;
-
-	if (substream->runtime->status->state == SNDRV_PCM_STATE_OPEN) {
-		unsigned int rate = params_rate(hw_params);
-
-		mutex_lock(&dice->mutex);
-		err = snd_dice_stream_reserve_duplex(dice, rate);
-		if (err >= 0)
-			++dice->substreams_counter;
-		mutex_unlock(&dice->mutex);
-	}
-
-	return err;
-}
-
-static int capture_hw_free(struct snd_pcm_substream *substream)
-{
-	struct snd_dice *dice = substream->private_data;
-
-	mutex_lock(&dice->mutex);
-
-	if (substream->runtime->status->state != SNDRV_PCM_STATE_OPEN)
-		--dice->substreams_counter;
-
-	snd_dice_stream_stop_duplex(dice);
-	snd_dice_stream_release_duplex(dice);
-
-	mutex_unlock(&dice->mutex);
-
-	return snd_pcm_lib_free_vmalloc_buffer(substream);
-}
 
-static int playback_hw_free(struct snd_pcm_substream *substream)
+static int pcm_hw_free(struct snd_pcm_substream *substream)
 {
 	struct snd_dice *dice = substream->private_data;
 
@@ -415,8 +375,8 @@ int snd_dice_create_pcm(struct snd_dice *dice)
 		.open      = pcm_open,
 		.close     = pcm_close,
 		.ioctl     = snd_pcm_lib_ioctl,
-		.hw_params = capture_hw_params,
-		.hw_free   = capture_hw_free,
+		.hw_params = pcm_hw_params,
+		.hw_free   = pcm_hw_free,
 		.prepare   = capture_prepare,
 		.trigger   = capture_trigger,
 		.pointer   = capture_pointer,
@@ -427,8 +387,8 @@ int snd_dice_create_pcm(struct snd_dice *dice)
 		.open      = pcm_open,
 		.close     = pcm_close,
 		.ioctl     = snd_pcm_lib_ioctl,
-		.hw_params = playback_hw_params,
-		.hw_free   = playback_hw_free,
+		.hw_params = pcm_hw_params,
+		.hw_free   = pcm_hw_free,
 		.prepare   = playback_prepare,
 		.trigger   = playback_trigger,
 		.pointer   = playback_pointer,
-- 
2.20.1

Re: [PATCH 00/12] ALSA: firewire-digi00x/dice: reserve/release isochronous resources in pcm.hw_params/hw_free callbacks

[Takashi Iwai]

On Tue, 11 Jun 2019 15:21:06 +0200,
Takashi Sakamoto wrote:
> 
> Hi,
> 
> This patchset is a part of series of patches for all of drivers in
> ALSA firewire stack to reserve/release isochronous resources in
> pcm.hw_params/hw_free callbacks.
> 
> In current implementation, the resources are reserved at the same time
> to start packet streaming, and released at the same time to stop packet
> streaming. However, once allocated, the resources are available
> independent of lifetime of each of packet streaming.
> 
> The isochronous resources are the resources of IEEE 1394 bus. On the
> other side of view, it's a kind of resources of hardware to maintain
> the bus (isochronous resource manager). For this kind of reservation and
> release, hw_params and hw_free operations are suitable in ALSA PCM
> interface.
> 
> Ideally, the operation to reserve/release isochronous resource should
> be separated from the operation to start/stop packet streaming. However,
> IEEE 1394 bus has reset event. Once reset occurs, isochronous resource
> manager releases allocated resources. The resources should be
> reallocated by requesters themselves. For this reason, in this patchset,
> bus generation is checked before starting packet streaming. If
> generation is updated, reallocation is requested to isochronous
> resource manager, then packet streaming starts.
> 
> Takashi Sakamoto (12):
>   ALSA: firewire-digi00x: refactoring to move timing of registration for
>     isochronous channel
>   ALSA: firewire-digi00x: code refactoring to finish streaming session
>   ALSA: firewire-digi00x: simplify error path to begin streaming session
>   ALSA: firewire-digi00x: code refactoring to keep isochronous resources
>   ALSA: firewire-digi00x: reserve/release isochronous resources in
>     pcm.hw_params/hw_free callbacks
>   ALSA: firewire-digi00x: update isochronous resources when starting
>     packet streaming after bus-reset
>   ALSA: firewire-digi00x: code refactoring for pcm.hw_params/hw_free
>     callbacks
>   ALSA: dice: code refactoring to stop packet streaming
>   ALSA: dice: code refactoring to keep isochronous resources
>   ALSA: dice: reserve/release isochronous resources in
>     pcm.hw_params/hw_free callbacks
>   ALSA: dice: update isochronous resources when starting packet
>     streaming after bus-reset
>   ALSA: dice: code refactoring for pcm.hw_params/hw_free callbacks

Applied all twelve patches.  Thanks.


Takashi