[PATCH 00/10] ALSA: firewire-motu: reserve/release isochronous resources in pcm.hw_params/hw_free callbacks

[PATCH 00/10] ALSA: firewire-motu: reserve/release isochronous resources in pcm.hw_params/hw_free callbacks

[Takashi Sakamoto]

Hi,

This patchset is the last part of patches for all of drivers in ALSA
firewire stack to reserve/release isochronous resources in
pcm.hw_params/hw_free callbacks, like the other patchsets:

https://mailman.alsa-project.org/pipermail/alsa-devel/2019-June/150118.html
https://mailman.alsa-project.org/pipermail/alsa-devel/2019-June/150863.html

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 (10):
  ALSA: firewire-motu: unify the count of subscriber for packet
    streaming
  ALSA: firewire-motu: unify midi callback function
  ALSA: firewire-motu: add helper function to keep isochronous resources
  ALSA: firewire-motu: code refactoring for condition to stop streaming
  ALSA: firewire-motu: rename helper functions to begin/finish streaming
    session
  ALSA: firewire-motu: minor code refactoring to stop isochronous
    context
  ALSA: firewire-motu: code refactoring to finish streaming session
  ALSA: firewire-motu: reserve/release isochronous resources in
    pcm.hw_params/hw_free callbacks
  ALSA: firewire-motu: update isochronous resources when starting packet
    streaming after bus-reset
  ALSA: firewire-motu: code refactoring for pcm.hw_params/hw_free
    callbacks

 sound/firewire/motu/motu-midi.c   |  59 ++-------
 sound/firewire/motu/motu-pcm.c    |  62 +++------
 sound/firewire/motu/motu-stream.c | 203 +++++++++++++++---------------
 sound/firewire/motu/motu.h        |   7 +-
 4 files changed, 135 insertions(+), 196 deletions(-)

-- 
2.20.1

[PATCH 01/10] ALSA: firewire-motu: unify the count of subscriber for packet streaming

[Takashi Sakamoto]

Two counters are used to maintain isochronous packet streaming for both
directions. However, like the other drivers, they can be replaced with
one counter. This commit unifies them.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/motu/motu-midi.c   |  8 ++++----
 sound/firewire/motu/motu-pcm.c    |  8 ++++----
 sound/firewire/motu/motu-stream.c | 17 ++++++-----------
 sound/firewire/motu/motu.h        |  3 +--
 4 files changed, 15 insertions(+), 21 deletions(-)

diff --git a/sound/firewire/motu/motu-midi.c b/sound/firewire/motu/motu-midi.c
index e55cab6d79c7..b2d339a8ef3f 100644
--- a/sound/firewire/motu/motu-midi.c
+++ b/sound/firewire/motu/motu-midi.c
@@ -18,7 +18,7 @@ static int midi_capture_open(struct snd_rawmidi_substream *substream)
 
 	mutex_lock(&motu->mutex);
 
-	motu->capture_substreams++;
+	motu->substreams_counter++;
 	err = snd_motu_stream_start_duplex(motu, 0);
 
 	mutex_unlock(&motu->mutex);
@@ -40,7 +40,7 @@ static int midi_playback_open(struct snd_rawmidi_substream *substream)
 
 	mutex_lock(&motu->mutex);
 
-	motu->playback_substreams++;
+	motu->substreams_counter++;
 	err = snd_motu_stream_start_duplex(motu, 0);
 
 	mutex_unlock(&motu->mutex);
@@ -57,7 +57,7 @@ static int midi_capture_close(struct snd_rawmidi_substream *substream)
 
 	mutex_lock(&motu->mutex);
 
-	motu->capture_substreams--;
+	motu->substreams_counter--;
 	snd_motu_stream_stop_duplex(motu);
 
 	mutex_unlock(&motu->mutex);
@@ -72,7 +72,7 @@ static int midi_playback_close(struct snd_rawmidi_substream *substream)
 
 	mutex_lock(&motu->mutex);
 
-	motu->playback_substreams--;
+	motu->substreams_counter--;
 	snd_motu_stream_stop_duplex(motu);
 
 	mutex_unlock(&motu->mutex);
diff --git a/sound/firewire/motu/motu-pcm.c b/sound/firewire/motu/motu-pcm.c
index ab69d7e6ac05..b0e5ebf05bec 100644
--- a/sound/firewire/motu/motu-pcm.c
+++ b/sound/firewire/motu/motu-pcm.c
@@ -203,7 +203,7 @@ static int capture_hw_params(struct snd_pcm_substream *substream,
 
 	if (substream->runtime->status->state == SNDRV_PCM_STATE_OPEN) {
 		mutex_lock(&motu->mutex);
-		motu->capture_substreams++;
+		motu->substreams_counter++;
 		mutex_unlock(&motu->mutex);
 	}
 
@@ -222,7 +222,7 @@ static int playback_hw_params(struct snd_pcm_substream *substream,
 
 	if (substream->runtime->status->state == SNDRV_PCM_STATE_OPEN) {
 		mutex_lock(&motu->mutex);
-		motu->playback_substreams++;
+		motu->substreams_counter++;
 		mutex_unlock(&motu->mutex);
 	}
 
@@ -236,7 +236,7 @@ static int capture_hw_free(struct snd_pcm_substream *substream)
 	mutex_lock(&motu->mutex);
 
 	if (substream->runtime->status->state != SNDRV_PCM_STATE_OPEN)
-		motu->capture_substreams--;
+		motu->substreams_counter--;
 
 	snd_motu_stream_stop_duplex(motu);
 
@@ -252,7 +252,7 @@ static int playback_hw_free(struct snd_pcm_substream *substream)
 	mutex_lock(&motu->mutex);
 
 	if (substream->runtime->status->state != SNDRV_PCM_STATE_OPEN)
-		motu->playback_substreams--;
+		motu->substreams_counter--;
 
 	snd_motu_stream_stop_duplex(motu);
 
diff --git a/sound/firewire/motu/motu-stream.c b/sound/firewire/motu/motu-stream.c
index 73e7a5e527fc..c136d7f8c49c 100644
--- a/sound/firewire/motu/motu-stream.c
+++ b/sound/firewire/motu/motu-stream.c
@@ -207,7 +207,7 @@ int snd_motu_stream_start_duplex(struct snd_motu *motu, unsigned int rate)
 	unsigned int curr_rate;
 	int err = 0;
 
-	if (motu->capture_substreams == 0 && motu->playback_substreams == 0)
+	if (motu->substreams_counter == 0)
 		return 0;
 
 	/* Some packet queueing errors. */
@@ -271,8 +271,7 @@ int snd_motu_stream_start_duplex(struct snd_motu *motu, unsigned int rate)
 		}
 	}
 
-	if (!amdtp_stream_running(&motu->tx_stream) &&
-	    motu->capture_substreams > 0) {
+	if (!amdtp_stream_running(&motu->tx_stream)) {
 		err = start_isoc_ctx(motu, &motu->tx_stream);
 		if (err < 0) {
 			dev_err(&motu->unit->device,
@@ -291,15 +290,12 @@ int snd_motu_stream_start_duplex(struct snd_motu *motu, unsigned int rate)
 
 void snd_motu_stream_stop_duplex(struct snd_motu *motu)
 {
-	if (motu->capture_substreams == 0) {
+	if (motu->substreams_counter == 0) {
 		if (amdtp_stream_running(&motu->tx_stream))
 			stop_isoc_ctx(motu, &motu->tx_stream);
 
-		if (motu->playback_substreams == 0) {
-			if (amdtp_stream_running(&motu->rx_stream))
-				stop_isoc_ctx(motu, &motu->rx_stream);
-			stop_both_streams(motu);
-		}
+		if (amdtp_stream_running(&motu->rx_stream))
+			stop_isoc_ctx(motu, &motu->rx_stream);
 	}
 }
 
@@ -372,8 +368,7 @@ void snd_motu_stream_destroy_duplex(struct snd_motu *motu)
 	destroy_stream(motu, AMDTP_IN_STREAM);
 	destroy_stream(motu, AMDTP_OUT_STREAM);
 
-	motu->playback_substreams = 0;
-	motu->capture_substreams = 0;
+	motu->substreams_counter = 0;
 }
 
 static void motu_lock_changed(struct snd_motu *motu)
diff --git a/sound/firewire/motu/motu.h b/sound/firewire/motu/motu.h
index 1cd112be7dad..ae4b37cdfade 100644
--- a/sound/firewire/motu/motu.h
+++ b/sound/firewire/motu/motu.h
@@ -60,8 +60,7 @@ struct snd_motu {
 	struct amdtp_stream rx_stream;
 	struct fw_iso_resources tx_resources;
 	struct fw_iso_resources rx_resources;
-	unsigned int capture_substreams;
-	unsigned int playback_substreams;
+	unsigned int substreams_counter;
 
 	/* For notification. */
 	struct fw_address_handler async_handler;
-- 
2.20.1

[PATCH 02/10] ALSA: firewire-motu: unify midi callback function

[Takashi Sakamoto]

Two sets of callbacks for rawmidi.open/close but they have the same
codes. This commit unifies each of the callbacks.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/motu/motu-midi.c | 49 ++++-----------------------------
 1 file changed, 6 insertions(+), 43 deletions(-)

diff --git a/sound/firewire/motu/motu-midi.c b/sound/firewire/motu/motu-midi.c
index b2d339a8ef3f..3b205386ed24 100644
--- a/sound/firewire/motu/motu-midi.c
+++ b/sound/firewire/motu/motu-midi.c
@@ -7,7 +7,7 @@
  */
 #include "motu.h"
 
-static int midi_capture_open(struct snd_rawmidi_substream *substream)
+static int midi_open(struct snd_rawmidi_substream *substream)
 {
 	struct snd_motu *motu = substream->rmidi->private_data;
 	int err;
@@ -29,44 +29,7 @@ static int midi_capture_open(struct snd_rawmidi_substream *substream)
 	return err;
 }
 
-static int midi_playback_open(struct snd_rawmidi_substream *substream)
-{
-	struct snd_motu *motu = substream->rmidi->private_data;
-	int err;
-
-	err = snd_motu_stream_lock_try(motu);
-	if (err < 0)
-		return err;
-
-	mutex_lock(&motu->mutex);
-
-	motu->substreams_counter++;
-	err = snd_motu_stream_start_duplex(motu, 0);
-
-	mutex_unlock(&motu->mutex);
-
-	if (err < 0)
-		snd_motu_stream_lock_release(motu);
-
-	return err;
-}
-
-static int midi_capture_close(struct snd_rawmidi_substream *substream)
-{
-	struct snd_motu *motu = substream->rmidi->private_data;
-
-	mutex_lock(&motu->mutex);
-
-	motu->substreams_counter--;
-	snd_motu_stream_stop_duplex(motu);
-
-	mutex_unlock(&motu->mutex);
-
-	snd_motu_stream_lock_release(motu);
-	return 0;
-}
-
-static int midi_playback_close(struct snd_rawmidi_substream *substream)
+static int midi_close(struct snd_rawmidi_substream *substream)
 {
 	struct snd_motu *motu = substream->rmidi->private_data;
 
@@ -129,13 +92,13 @@ static void set_midi_substream_names(struct snd_motu *motu,
 int snd_motu_create_midi_devices(struct snd_motu *motu)
 {
 	static const struct snd_rawmidi_ops capture_ops = {
-		.open		= midi_capture_open,
-		.close		= midi_capture_close,
+		.open		= midi_open,
+		.close		= midi_close,
 		.trigger	= midi_capture_trigger,
 	};
 	static const struct snd_rawmidi_ops playback_ops = {
-		.open		= midi_playback_open,
-		.close		= midi_playback_close,
+		.open		= midi_open,
+		.close		= midi_close,
 		.trigger	= midi_playback_trigger,
 	};
 	struct snd_rawmidi *rmidi;
-- 
2.20.1

[PATCH 03/10] ALSA: firewire-motu: add helper function 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.

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

diff --git a/sound/firewire/motu/motu-stream.c b/sound/firewire/motu/motu-stream.c
index c136d7f8c49c..35b4e0f72fe2 100644
--- a/sound/firewire/motu/motu-stream.c
+++ b/sound/firewire/motu/motu-stream.c
@@ -26,48 +26,55 @@
 #define  RX_PACKET_EXCLUDE_DIFFERED_DATA_CHUNKS	0x00000040
 #define  TX_PACKET_TRANSMISSION_SPEED_MASK	0x0000000f
 
-static int start_both_streams(struct snd_motu *motu, unsigned int rate)
+static int keep_resources(struct snd_motu *motu, unsigned int rate,
+			  struct amdtp_stream *stream)
 {
+	struct fw_iso_resources *resources;
+	struct snd_motu_packet_format *packet_format;
 	unsigned int midi_ports = 0;
-	__be32 reg;
-	u32 data;
 	int err;
 
-	if ((motu->spec->flags & SND_MOTU_SPEC_RX_MIDI_2ND_Q) ||
-	    (motu->spec->flags & SND_MOTU_SPEC_RX_MIDI_3RD_Q))
-		midi_ports = 1;
+	if (stream == &motu->rx_stream) {
+		resources = &motu->rx_resources;
+		packet_format = &motu->rx_packet_formats;
 
-	/* Set packet formation to our packet streaming engine. */
-	err = amdtp_motu_set_parameters(&motu->rx_stream, rate, midi_ports,
-					&motu->rx_packet_formats);
-	if (err < 0)
-		return err;
+		if ((motu->spec->flags & SND_MOTU_SPEC_RX_MIDI_2ND_Q) ||
+		    (motu->spec->flags & SND_MOTU_SPEC_RX_MIDI_3RD_Q))
+			midi_ports = 1;
+	} else {
+		resources = &motu->tx_resources;
+		packet_format = &motu->tx_packet_formats;
 
-	if ((motu->spec->flags & SND_MOTU_SPEC_TX_MIDI_2ND_Q) ||
-	    (motu->spec->flags & SND_MOTU_SPEC_TX_MIDI_3RD_Q))
-		midi_ports = 1;
-	else
-		midi_ports = 0;
+		if ((motu->spec->flags & SND_MOTU_SPEC_TX_MIDI_2ND_Q) ||
+		    (motu->spec->flags & SND_MOTU_SPEC_TX_MIDI_3RD_Q))
+			midi_ports = 1;
+	}
 
-	err = amdtp_motu_set_parameters(&motu->tx_stream, rate, midi_ports,
-					&motu->tx_packet_formats);
+	err = amdtp_motu_set_parameters(stream, rate, midi_ports,
+					packet_format);
 	if (err < 0)
 		return err;
 
-	/* Get isochronous resources on the bus. */
-	err = fw_iso_resources_allocate(&motu->rx_resources,
-				amdtp_stream_get_max_payload(&motu->rx_stream),
+	return fw_iso_resources_allocate(resources,
+				amdtp_stream_get_max_payload(stream),
 				fw_parent_device(motu->unit)->max_speed);
+}
+
+static int start_both_streams(struct snd_motu *motu, unsigned int rate)
+{
+	__be32 reg;
+	u32 data;
+	int err;
+
+	err = keep_resources(motu, rate, &motu->tx_stream);
 	if (err < 0)
 		return err;
 
-	err = fw_iso_resources_allocate(&motu->tx_resources,
-				amdtp_stream_get_max_payload(&motu->tx_stream),
-				fw_parent_device(motu->unit)->max_speed);
+	err = keep_resources(motu, rate, &motu->rx_stream);
 	if (err < 0)
 		return err;
 
-	/* Configure the unit to start isochronous communication. */
+	// Configure the unit to start isochronous communication.
 	err = snd_motu_transaction_read(motu, ISOC_COMM_CONTROL_OFFSET, &reg,
 					sizeof(reg));
 	if (err < 0)
-- 
2.20.1

[PATCH 04/10] ALSA: firewire-motu: code refactoring for condition to stop streaming

[Takashi Sakamoto]

In snd_motu_stream_start_duplex() function, there're two points to stop
packet streaming, however they can be merged.  This commit merges them.

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

diff --git a/sound/firewire/motu/motu-stream.c b/sound/firewire/motu/motu-stream.c
index 35b4e0f72fe2..a3f45f2c817c 100644
--- a/sound/firewire/motu/motu-stream.c
+++ b/sound/firewire/motu/motu-stream.c
@@ -217,19 +217,11 @@ int snd_motu_stream_start_duplex(struct snd_motu *motu, unsigned int rate)
 	if (motu->substreams_counter == 0)
 		return 0;
 
-	/* Some packet queueing errors. */
-	if (amdtp_streaming_error(&motu->rx_stream) ||
-	    amdtp_streaming_error(&motu->tx_stream)) {
-		amdtp_stream_stop(&motu->rx_stream);
-		amdtp_stream_stop(&motu->tx_stream);
-		stop_both_streams(motu);
-	}
-
 	err = snd_motu_stream_cache_packet_formats(motu);
 	if (err < 0)
 		return err;
 
-	/* Stop stream if rate is different. */
+	// Stop stream if rate is different.
 	err = protocol->get_clock_rate(motu, &curr_rate);
 	if (err < 0) {
 		dev_err(&motu->unit->device,
@@ -238,7 +230,9 @@ int snd_motu_stream_start_duplex(struct snd_motu *motu, unsigned int rate)
 	}
 	if (rate == 0)
 		rate = curr_rate;
-	if (rate != curr_rate) {
+	if (rate != curr_rate ||
+	    amdtp_streaming_error(&motu->rx_stream) ||
+	    amdtp_streaming_error(&motu->tx_stream)) {
 		amdtp_stream_stop(&motu->rx_stream);
 		amdtp_stream_stop(&motu->tx_stream);
 		stop_both_streams(motu);
-- 
2.20.1

[PATCH 05/10] ALSA: firewire-motu: rename helper functions to begin/finish streaming session

[Takashi Sakamoto]

Like the other drivers in ALSA firewire stack, this commit renames
functions to begin/finish packet streaming.

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

diff --git a/sound/firewire/motu/motu-stream.c b/sound/firewire/motu/motu-stream.c
index a3f45f2c817c..cb01f1d784f1 100644
--- a/sound/firewire/motu/motu-stream.c
+++ b/sound/firewire/motu/motu-stream.c
@@ -60,7 +60,7 @@ static int keep_resources(struct snd_motu *motu, unsigned int rate,
 				fw_parent_device(motu->unit)->max_speed);
 }
 
-static int start_both_streams(struct snd_motu *motu, unsigned int rate)
+static int begin_session(struct snd_motu *motu, unsigned int rate)
 {
 	__be32 reg;
 	u32 data;
@@ -91,7 +91,7 @@ static int start_both_streams(struct snd_motu *motu, unsigned int rate)
 					  sizeof(reg));
 }
 
-static void stop_both_streams(struct snd_motu *motu)
+static void finish_session(struct snd_motu *motu)
 {
 	__be32 reg;
 	u32 data;
@@ -235,7 +235,7 @@ int snd_motu_stream_start_duplex(struct snd_motu *motu, unsigned int rate)
 	    amdtp_streaming_error(&motu->tx_stream)) {
 		amdtp_stream_stop(&motu->rx_stream);
 		amdtp_stream_stop(&motu->tx_stream);
-		stop_both_streams(motu);
+		finish_session(motu);
 	}
 
 	if (!amdtp_stream_running(&motu->rx_stream)) {
@@ -250,7 +250,7 @@ int snd_motu_stream_start_duplex(struct snd_motu *motu, unsigned int rate)
 		if (err < 0)
 			return err;
 
-		err = start_both_streams(motu, rate);
+		err = begin_session(motu, rate);
 		if (err < 0) {
 			dev_err(&motu->unit->device,
 				"fail to start isochronous comm: %d\n", err);
@@ -285,7 +285,7 @@ int snd_motu_stream_start_duplex(struct snd_motu *motu, unsigned int rate)
 	return 0;
 
 stop_streams:
-	stop_both_streams(motu);
+	finish_session(motu);
 	return err;
 }
 
-- 
2.20.1

[PATCH 06/10] ALSA: firewire-motu: minor code refactoring to stop isochronous context

[Takashi Sakamoto]

The helper function to stop isochronous context is superfluous. This
commit removes it with simpler codes.

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

diff --git a/sound/firewire/motu/motu-stream.c b/sound/firewire/motu/motu-stream.c
index cb01f1d784f1..4fbec35eaad5 100644
--- a/sound/firewire/motu/motu-stream.c
+++ b/sound/firewire/motu/motu-stream.c
@@ -142,19 +142,6 @@ static int start_isoc_ctx(struct snd_motu *motu, struct amdtp_stream *stream)
 	return 0;
 }
 
-static void stop_isoc_ctx(struct snd_motu *motu, struct amdtp_stream *stream)
-{
-	struct fw_iso_resources *resources;
-
-	if (stream == &motu->rx_stream)
-		resources = &motu->rx_resources;
-	else
-		resources = &motu->tx_resources;
-
-	amdtp_stream_stop(stream);
-	fw_iso_resources_free(resources);
-}
-
 int snd_motu_stream_cache_packet_formats(struct snd_motu *motu)
 {
 	int err;
@@ -292,11 +279,11 @@ int snd_motu_stream_start_duplex(struct snd_motu *motu, unsigned int rate)
 void snd_motu_stream_stop_duplex(struct snd_motu *motu)
 {
 	if (motu->substreams_counter == 0) {
-		if (amdtp_stream_running(&motu->tx_stream))
-			stop_isoc_ctx(motu, &motu->tx_stream);
+		amdtp_stream_stop(&motu->tx_stream);
+		amdtp_stream_stop(&motu->rx_stream);
 
-		if (amdtp_stream_running(&motu->rx_stream))
-			stop_isoc_ctx(motu, &motu->rx_stream);
+		fw_iso_resources_free(&motu->tx_resources);
+		fw_iso_resources_free(&motu->rx_resources);
 	}
 }
 
-- 
2.20.1

[PATCH 07/10] ALSA: firewire-motu: code refactoring to finish streaming session

[Takashi Sakamoto]

The operation to finish packet streaming includes stopping isochronous
contexts. This commit merges it to the helper function.

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

diff --git a/sound/firewire/motu/motu-stream.c b/sound/firewire/motu/motu-stream.c
index 4fbec35eaad5..53c43848b137 100644
--- a/sound/firewire/motu/motu-stream.c
+++ b/sound/firewire/motu/motu-stream.c
@@ -101,6 +101,9 @@ static void finish_session(struct snd_motu *motu)
 	if (err < 0)
 		return;
 
+	amdtp_stream_stop(&motu->tx_stream);
+	amdtp_stream_stop(&motu->rx_stream);
+
 	err = snd_motu_transaction_read(motu, ISOC_COMM_CONTROL_OFFSET, &reg,
 					sizeof(reg));
 	if (err < 0)
@@ -219,11 +222,8 @@ int snd_motu_stream_start_duplex(struct snd_motu *motu, unsigned int rate)
 		rate = curr_rate;
 	if (rate != curr_rate ||
 	    amdtp_streaming_error(&motu->rx_stream) ||
-	    amdtp_streaming_error(&motu->tx_stream)) {
-		amdtp_stream_stop(&motu->rx_stream);
-		amdtp_stream_stop(&motu->tx_stream);
+	    amdtp_streaming_error(&motu->tx_stream))
 		finish_session(motu);
-	}
 
 	if (!amdtp_stream_running(&motu->rx_stream)) {
 		err = protocol->set_clock_rate(motu, rate);
@@ -278,13 +278,8 @@ int snd_motu_stream_start_duplex(struct snd_motu *motu, unsigned int rate)
 
 void snd_motu_stream_stop_duplex(struct snd_motu *motu)
 {
-	if (motu->substreams_counter == 0) {
-		amdtp_stream_stop(&motu->tx_stream);
-		amdtp_stream_stop(&motu->rx_stream);
-
-		fw_iso_resources_free(&motu->tx_resources);
-		fw_iso_resources_free(&motu->rx_resources);
-	}
+	if (motu->substreams_counter == 0)
+		finish_session(motu);
 }
 
 static int init_stream(struct snd_motu *motu, enum amdtp_stream_direction dir)
-- 
2.20.1

[PATCH 08/10] ALSA: firewire-motu: 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/motu/motu-midi.c   |  10 ++-
 sound/firewire/motu/motu-pcm.c    |  26 +++++---
 sound/firewire/motu/motu-stream.c | 100 +++++++++++++++++-------------
 sound/firewire/motu/motu.h        |   4 +-
 4 files changed, 84 insertions(+), 56 deletions(-)

diff --git a/sound/firewire/motu/motu-midi.c b/sound/firewire/motu/motu-midi.c
index 3b205386ed24..a463730c72bc 100644
--- a/sound/firewire/motu/motu-midi.c
+++ b/sound/firewire/motu/motu-midi.c
@@ -18,8 +18,11 @@ static int midi_open(struct snd_rawmidi_substream *substream)
 
 	mutex_lock(&motu->mutex);
 
-	motu->substreams_counter++;
-	err = snd_motu_stream_start_duplex(motu, 0);
+	err = snd_motu_stream_reserve_duplex(motu, 0);
+	if (err >= 0) {
+		++motu->substreams_counter;
+		err = snd_motu_stream_start_duplex(motu);
+	}
 
 	mutex_unlock(&motu->mutex);
 
@@ -35,8 +38,9 @@ static int midi_close(struct snd_rawmidi_substream *substream)
 
 	mutex_lock(&motu->mutex);
 
-	motu->substreams_counter--;
+	--motu->substreams_counter;
 	snd_motu_stream_stop_duplex(motu);
+	snd_motu_stream_release_duplex(motu);
 
 	mutex_unlock(&motu->mutex);
 
diff --git a/sound/firewire/motu/motu-pcm.c b/sound/firewire/motu/motu-pcm.c
index b0e5ebf05bec..d4e75d3ee928 100644
--- a/sound/firewire/motu/motu-pcm.c
+++ b/sound/firewire/motu/motu-pcm.c
@@ -202,12 +202,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(&motu->mutex);
-		motu->substreams_counter++;
+		err = snd_motu_stream_reserve_duplex(motu, rate);
+		if (err >= 0)
+			++motu->substreams_counter;
 		mutex_unlock(&motu->mutex);
 	}
 
-	return 0;
+	return err;
 }
 static int playback_hw_params(struct snd_pcm_substream *substream,
 			      struct snd_pcm_hw_params *hw_params)
@@ -221,12 +225,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(&motu->mutex);
-		motu->substreams_counter++;
+		err = snd_motu_stream_reserve_duplex(motu, rate);
+		if (err >= 0)
+			++motu->substreams_counter;
 		mutex_unlock(&motu->mutex);
 	}
 
-	return 0;
+	return err;
 }
 
 static int capture_hw_free(struct snd_pcm_substream *substream)
@@ -236,9 +244,10 @@ static int capture_hw_free(struct snd_pcm_substream *substream)
 	mutex_lock(&motu->mutex);
 
 	if (substream->runtime->status->state != SNDRV_PCM_STATE_OPEN)
-		motu->substreams_counter--;
+		--motu->substreams_counter;
 
 	snd_motu_stream_stop_duplex(motu);
+	snd_motu_stream_release_duplex(motu);
 
 	mutex_unlock(&motu->mutex);
 
@@ -252,9 +261,10 @@ static int playback_hw_free(struct snd_pcm_substream *substream)
 	mutex_lock(&motu->mutex);
 
 	if (substream->runtime->status->state != SNDRV_PCM_STATE_OPEN)
-		motu->substreams_counter--;
+		--motu->substreams_counter;
 
 	snd_motu_stream_stop_duplex(motu);
+	snd_motu_stream_release_duplex(motu);
 
 	mutex_unlock(&motu->mutex);
 
@@ -267,7 +277,7 @@ static int capture_prepare(struct snd_pcm_substream *substream)
 	int err;
 
 	mutex_lock(&motu->mutex);
-	err = snd_motu_stream_start_duplex(motu, substream->runtime->rate);
+	err = snd_motu_stream_start_duplex(motu);
 	mutex_unlock(&motu->mutex);
 	if (err >= 0)
 		amdtp_stream_pcm_prepare(&motu->tx_stream);
@@ -280,7 +290,7 @@ static int playback_prepare(struct snd_pcm_substream *substream)
 	int err;
 
 	mutex_lock(&motu->mutex);
-	err = snd_motu_stream_start_duplex(motu, substream->runtime->rate);
+	err = snd_motu_stream_start_duplex(motu);
 	mutex_unlock(&motu->mutex);
 	if (err >= 0)
 		amdtp_stream_pcm_prepare(&motu->rx_stream);
diff --git a/sound/firewire/motu/motu-stream.c b/sound/firewire/motu/motu-stream.c
index 53c43848b137..5630a8f1f2f1 100644
--- a/sound/firewire/motu/motu-stream.c
+++ b/sound/firewire/motu/motu-stream.c
@@ -60,20 +60,12 @@ static int keep_resources(struct snd_motu *motu, unsigned int rate,
 				fw_parent_device(motu->unit)->max_speed);
 }
 
-static int begin_session(struct snd_motu *motu, unsigned int rate)
+static int begin_session(struct snd_motu *motu)
 {
 	__be32 reg;
 	u32 data;
 	int err;
 
-	err = keep_resources(motu, rate, &motu->tx_stream);
-	if (err < 0)
-		return err;
-
-	err = keep_resources(motu, rate, &motu->rx_stream);
-	if (err < 0)
-		return err;
-
 	// Configure the unit to start isochronous communication.
 	err = snd_motu_transaction_read(motu, ISOC_COMM_CONTROL_OFFSET, &reg,
 					sizeof(reg));
@@ -116,9 +108,6 @@ static void finish_session(struct snd_motu *motu)
 	reg = cpu_to_be32(data);
 	snd_motu_transaction_write(motu, ISOC_COMM_CONTROL_OFFSET, &reg,
 				   sizeof(reg));
-
-	fw_iso_resources_free(&motu->tx_resources);
-	fw_iso_resources_free(&motu->rx_resources);
 }
 
 static int start_isoc_ctx(struct snd_motu *motu, struct amdtp_stream *stream)
@@ -136,11 +125,8 @@ static int start_isoc_ctx(struct snd_motu *motu, struct amdtp_stream *stream)
 	if (err < 0)
 		return err;
 
-	if (!amdtp_stream_wait_callback(stream, CALLBACK_TIMEOUT)) {
-		amdtp_stream_stop(stream);
-		fw_iso_resources_free(resources);
+	if (!amdtp_stream_wait_callback(stream, CALLBACK_TIMEOUT))
 		return -ETIMEDOUT;
-	}
 
 	return 0;
 }
@@ -172,6 +158,56 @@ int snd_motu_stream_cache_packet_formats(struct snd_motu *motu)
 	return 0;
 }
 
+int snd_motu_stream_reserve_duplex(struct snd_motu *motu, unsigned int rate)
+{
+	unsigned int curr_rate;
+	int err;
+
+	err = motu->spec->protocol->get_clock_rate(motu, &curr_rate);
+	if (err < 0)
+		return err;
+	if (rate == 0)
+		rate = curr_rate;
+
+	if (motu->substreams_counter == 0 || curr_rate != rate) {
+		finish_session(motu);
+
+		fw_iso_resources_free(&motu->tx_resources);
+		fw_iso_resources_free(&motu->rx_resources);
+
+		err = motu->spec->protocol->set_clock_rate(motu, rate);
+		if (err < 0) {
+			dev_err(&motu->unit->device,
+				"fail to set sampling rate: %d\n", err);
+			return err;
+		}
+
+		err = snd_motu_stream_cache_packet_formats(motu);
+		if (err < 0)
+			return err;
+
+		err = keep_resources(motu, rate, &motu->tx_stream);
+		if (err < 0)
+			return err;
+
+		err = keep_resources(motu, rate, &motu->rx_stream);
+		if (err < 0) {
+			fw_iso_resources_free(&motu->tx_resources);
+			return err;
+		}
+	}
+
+	return 0;
+}
+
+void snd_motu_stream_release_duplex(struct snd_motu *motu)
+{
+	if (motu->substreams_counter == 0) {
+		fw_iso_resources_free(&motu->tx_resources);
+		fw_iso_resources_free(&motu->rx_resources);
+	}
+}
+
 static int ensure_packet_formats(struct snd_motu *motu)
 {
 	__be32 reg;
@@ -198,46 +234,23 @@ static int ensure_packet_formats(struct snd_motu *motu)
 					  sizeof(reg));
 }
 
-int snd_motu_stream_start_duplex(struct snd_motu *motu, unsigned int rate)
+int snd_motu_stream_start_duplex(struct snd_motu *motu)
 {
-	const struct snd_motu_protocol *protocol = motu->spec->protocol;
-	unsigned int curr_rate;
 	int err = 0;
 
 	if (motu->substreams_counter == 0)
 		return 0;
 
-	err = snd_motu_stream_cache_packet_formats(motu);
-	if (err < 0)
-		return err;
-
-	// Stop stream if rate is different.
-	err = protocol->get_clock_rate(motu, &curr_rate);
-	if (err < 0) {
-		dev_err(&motu->unit->device,
-			"fail to get sampling rate: %d\n", err);
-		return err;
-	}
-	if (rate == 0)
-		rate = curr_rate;
-	if (rate != curr_rate ||
-	    amdtp_streaming_error(&motu->rx_stream) ||
+	if (amdtp_streaming_error(&motu->rx_stream) ||
 	    amdtp_streaming_error(&motu->tx_stream))
 		finish_session(motu);
 
 	if (!amdtp_stream_running(&motu->rx_stream)) {
-		err = protocol->set_clock_rate(motu, rate);
-		if (err < 0) {
-			dev_err(&motu->unit->device,
-				"fail to set sampling rate: %d\n", err);
-			return err;
-		}
-
 		err = ensure_packet_formats(motu);
 		if (err < 0)
 			return err;
 
-		err = begin_session(motu, rate);
+		err = begin_session(motu);
 		if (err < 0) {
 			dev_err(&motu->unit->device,
 				"fail to start isochronous comm: %d\n", err);
@@ -251,7 +264,7 @@ int snd_motu_stream_start_duplex(struct snd_motu *motu, unsigned int rate)
 			goto stop_streams;
 		}
 
-		err = protocol->switch_fetching_mode(motu, true);
+		err = motu->spec->protocol->switch_fetching_mode(motu, true);
 		if (err < 0) {
 			dev_err(&motu->unit->device,
 				"fail to enable frame fetching: %d\n", err);
@@ -264,7 +277,6 @@ int snd_motu_stream_start_duplex(struct snd_motu *motu, unsigned int rate)
 		if (err < 0) {
 			dev_err(&motu->unit->device,
 				"fail to start IR context: %d", err);
-			amdtp_stream_stop(&motu->rx_stream);
 			goto stop_streams;
 		}
 	}
diff --git a/sound/firewire/motu/motu.h b/sound/firewire/motu/motu.h
index ae4b37cdfade..32cd42873fd0 100644
--- a/sound/firewire/motu/motu.h
+++ b/sound/firewire/motu/motu.h
@@ -153,7 +153,9 @@ void snd_motu_transaction_unregister(struct snd_motu *motu);
 int snd_motu_stream_init_duplex(struct snd_motu *motu);
 void snd_motu_stream_destroy_duplex(struct snd_motu *motu);
 int snd_motu_stream_cache_packet_formats(struct snd_motu *motu);
-int snd_motu_stream_start_duplex(struct snd_motu *motu, unsigned int rate);
+int snd_motu_stream_reserve_duplex(struct snd_motu *motu, unsigned int rate);
+void snd_motu_stream_release_duplex(struct snd_motu *motu);
+int snd_motu_stream_start_duplex(struct snd_motu *motu);
 void snd_motu_stream_stop_duplex(struct snd_motu *motu);
 int snd_motu_stream_lock_try(struct snd_motu *motu);
 void snd_motu_stream_lock_release(struct snd_motu *motu);
-- 
2.20.1

[PATCH 09/10] ALSA: firewire-motu: 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/motu/motu-stream.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/sound/firewire/motu/motu-stream.c b/sound/firewire/motu/motu-stream.c
index 5630a8f1f2f1..82891583b736 100644
--- a/sound/firewire/motu/motu-stream.c
+++ b/sound/firewire/motu/motu-stream.c
@@ -236,6 +236,7 @@ static int ensure_packet_formats(struct snd_motu *motu)
 
 int snd_motu_stream_start_duplex(struct snd_motu *motu)
 {
+	unsigned int generation = motu->rx_resources.generation;
 	int err = 0;
 
 	if (motu->substreams_counter == 0)
@@ -245,6 +246,16 @@ int snd_motu_stream_start_duplex(struct snd_motu *motu)
 	    amdtp_streaming_error(&motu->tx_stream))
 		finish_session(motu);
 
+	if (generation != fw_parent_device(motu->unit)->card->generation) {
+		err = fw_iso_resources_update(&motu->rx_resources);
+		if (err < 0)
+			return err;
+
+		err = fw_iso_resources_update(&motu->tx_resources);
+		if (err < 0)
+			return err;
+	}
+
 	if (!amdtp_stream_running(&motu->rx_stream)) {
 		err = ensure_packet_formats(motu);
 		if (err < 0)
-- 
2.20.1

[PATCH 10/10] ALSA: firewire-motu: 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/motu/motu-pcm.c | 54 +++++-----------------------------
 1 file changed, 7 insertions(+), 47 deletions(-)

diff --git a/sound/firewire/motu/motu-pcm.c b/sound/firewire/motu/motu-pcm.c
index d4e75d3ee928..b9852c911b98 100644
--- a/sound/firewire/motu/motu-pcm.c
+++ b/sound/firewire/motu/motu-pcm.c
@@ -190,8 +190,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_motu *motu = substream->private_data;
 	int err;
@@ -213,48 +213,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_motu *motu = 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(&motu->mutex);
-		err = snd_motu_stream_reserve_duplex(motu, rate);
-		if (err >= 0)
-			++motu->substreams_counter;
-		mutex_unlock(&motu->mutex);
-	}
-
-	return err;
-}
-
-static int capture_hw_free(struct snd_pcm_substream *substream)
-{
-	struct snd_motu *motu = substream->private_data;
-
-	mutex_lock(&motu->mutex);
-
-	if (substream->runtime->status->state != SNDRV_PCM_STATE_OPEN)
-		--motu->substreams_counter;
-
-	snd_motu_stream_stop_duplex(motu);
-	snd_motu_stream_release_duplex(motu);
-
-	mutex_unlock(&motu->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_motu *motu = substream->private_data;
 
@@ -366,8 +326,8 @@ int snd_motu_create_pcm_devices(struct snd_motu *motu)
 		.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,
@@ -378,8 +338,8 @@ int snd_motu_create_pcm_devices(struct snd_motu *motu)
 		.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/10] ALSA: firewire-motu: reserve/release isochronous resources in pcm.hw_params/hw_free callbacks

[Takashi Iwai]

On Mon, 17 Jun 2019 10:15:00 +0200,
Takashi Sakamoto wrote:
> 
> Hi,
> 
> This patchset is the last part of patches for all of drivers in ALSA
> firewire stack to reserve/release isochronous resources in
> pcm.hw_params/hw_free callbacks, like the other patchsets:
> 
> https://mailman.alsa-project.org/pipermail/alsa-devel/2019-June/150118.html
> https://mailman.alsa-project.org/pipermail/alsa-devel/2019-June/150863.html
> 
> 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 (10):
>   ALSA: firewire-motu: unify the count of subscriber for packet
>     streaming
>   ALSA: firewire-motu: unify midi callback function
>   ALSA: firewire-motu: add helper function to keep isochronous resources
>   ALSA: firewire-motu: code refactoring for condition to stop streaming
>   ALSA: firewire-motu: rename helper functions to begin/finish streaming
>     session
>   ALSA: firewire-motu: minor code refactoring to stop isochronous
>     context
>   ALSA: firewire-motu: code refactoring to finish streaming session
>   ALSA: firewire-motu: reserve/release isochronous resources in
>     pcm.hw_params/hw_free callbacks
>   ALSA: firewire-motu: update isochronous resources when starting packet
>     streaming after bus-reset
>   ALSA: firewire-motu: code refactoring for pcm.hw_params/hw_free
>     callbacks

Applied all ten patches now.  Thanks.


Takashi