[alsa-devel] [PATCH 0/3] ALSA: bebob: misc improvements for starting/stopping packet streaming

[alsa-devel] [PATCH 0/3] ALSA: bebob: misc improvements for starting/stopping packet streaming

[Takashi Sakamoto]

Hi,

This patchset improves model-specific issues in a point of protocol
version supported by the model. The changes are relevant to packet
streaming, thus it can brings some regressions.

For testers, I prepare for a remote branch to backport the latest
codes to Linux v4.17 or later. I'm happy to receive any report.
https://github.com/takaswie/snd-firewire-improve/tree/topic/bebob-connection-issue

Takashi Sakamoto (3):
  ALSA: bebob: expand sleep just after breaking connections for protocol
    version 1
  ALSA: bebob: expand delay of start for IR context just for version 3
    firmware
  ALSA: bebob: link the order of establishing connections and Syt-match
    clock mode

 sound/firewire/bebob/bebob_stream.c | 76 ++++++++++++++---------------
 1 file changed, 38 insertions(+), 38 deletions(-)

-- 
2.20.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[alsa-devel] [PATCH 1/3] ALSA: bebob: expand sleep just after breaking connections for protocol version 1

[Takashi Sakamoto]

As long as I investigated, some devices with BeBoB protocol version 1
can be freezed during several hundreds milliseconds after breaking
connections. When accessing during the freezed time, any transaction
is corrupted. In the worst case, the device is going to reboot.

I can see this issue in:
 * Roland FA-66
 * M-Audio FireWire Solo

This commit expands sleep just after breaking connections to avoid
the freezed time as much as possible. I note that the freeze/reboot
behaviour is similar to below models:
 * Focusrite Saffire Pro 10 I/O
 * Focusrite Saffire Pro 26 I/O

The above models certainly reboot after breaking connections.

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

diff --git a/sound/firewire/bebob/bebob_stream.c b/sound/firewire/bebob/bebob_stream.c
index f7f0db5aa811..1b264d0d63eb 100644
--- a/sound/firewire/bebob/bebob_stream.c
+++ b/sound/firewire/bebob/bebob_stream.c
@@ -415,15 +415,16 @@ static int make_both_connections(struct snd_bebob *bebob)
 	return 0;
 }
 
-static void
-break_both_connections(struct snd_bebob *bebob)
+static void break_both_connections(struct snd_bebob *bebob)
 {
 	cmp_connection_break(&bebob->in_conn);
 	cmp_connection_break(&bebob->out_conn);
 
-	/* These models seems to be in transition state for a longer time. */
-	if (bebob->maudio_special_quirk != NULL)
-		msleep(200);
+	// These models seem to be in transition state for a longer time. When
+	// accessing in the state, any transactions is corrupted. In the worst
+	// case, the device is going to reboot.
+	if (bebob->version < 2)
+		msleep(600);
 }
 
 static int
-- 
2.20.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] [PATCH 1/3] ALSA: bebob: expand sleep just after breaking connections for protocol version 1

[Steve Morris]

On Fri, Nov 01, 2019 at 10:13:21PM +0900, Takashi Sakamoto wrote:
> As long as I investigated, some devices with BeBoB protocol version 1
> can be freezed during several hundreds milliseconds after breaking
> connections. When accessing during the freezed time, any transaction
> is corrupted. In the worst case, the device is going to reboot.
> 
> I can see this issue in:
>  * Roland FA-66
>  * M-Audio FireWire Solo
> 
> This commit expands sleep just after breaking connections to avoid
> the freezed time as much as possible. I note that the freeze/reboot
> behaviour is similar to below models:
>  * Focusrite Saffire Pro 10 I/O
>  * Focusrite Saffire Pro 26 I/O
> 
> The above models certainly reboot after breaking connections.
> 
> Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
> ---
>  sound/firewire/bebob/bebob_stream.c | 11 ++++++-----
>  1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/sound/firewire/bebob/bebob_stream.c b/sound/firewire/bebob/bebob_stream.c
> index f7f0db5aa811..1b264d0d63eb 100644
> --- a/sound/firewire/bebob/bebob_stream.c
> +++ b/sound/firewire/bebob/bebob_stream.c
> @@ -415,15 +415,16 @@ static int make_both_connections(struct snd_bebob *bebob)
>  	return 0;
>  }
>  
> -static void
> -break_both_connections(struct snd_bebob *bebob)
> +static void break_both_connections(struct snd_bebob *bebob)
>  {
>  	cmp_connection_break(&bebob->in_conn);
>  	cmp_connection_break(&bebob->out_conn);
>  
> -	/* These models seems to be in transition state for a longer time. */
> -	if (bebob->maudio_special_quirk != NULL)
> -		msleep(200);
> +	// These models seem to be in transition state for a longer time. When
> +	// accessing in the state, any transactions is corrupted. In the worst
> +	// case, the device is going to reboot.
> +	if (bebob->version < 2)
> +		msleep(600);
>  }
>  
>  static int
> -- 
> 2.20.1
> 
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

This patch causes a reboot when I power on my Edirol FA-101. This is
consistent behavior with kernels 5.5.1 - 5.5.11.

I've changed msleep(600) back to msleep(200) and rebuilt the module and
I am once again able to power on and use the interface.

I am running Arch linux with 5.5.11-arch1-1 #1 SMP PREEMPT Sun, 22 Mar 2020 16:33:15 +0000 x86_64 GNU/Linux.


I had previously posted to the list on Feb 19, 2020 with the subject:

sytem reboots when initializing Edirol FA-101 firewire audio interface

Please forgive any lapses in reporting protocol as I generally don't
work at the kernel level. Hopefully this more specific report will help
fix the issue.

Thanks, 

Steve

Re: [alsa-devel] [PATCH 1/3] ALSA: bebob: expand sleep just after breaking connections for protocol version 1

[Takashi Iwai]

On Thu, 26 Mar 2020 22:47:22 +0100,
Steve Morris wrote:
> 
> On Fri, Nov 01, 2019 at 10:13:21PM +0900, Takashi Sakamoto wrote:
> > As long as I investigated, some devices with BeBoB protocol version 1
> > can be freezed during several hundreds milliseconds after breaking
> > connections. When accessing during the freezed time, any transaction
> > is corrupted. In the worst case, the device is going to reboot.
> > 
> > I can see this issue in:
> >  * Roland FA-66
> >  * M-Audio FireWire Solo
> > 
> > This commit expands sleep just after breaking connections to avoid
> > the freezed time as much as possible. I note that the freeze/reboot
> > behaviour is similar to below models:
> >  * Focusrite Saffire Pro 10 I/O
> >  * Focusrite Saffire Pro 26 I/O
> > 
> > The above models certainly reboot after breaking connections.
> > 
> > Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
> > ---
> >  sound/firewire/bebob/bebob_stream.c | 11 ++++++-----
> >  1 file changed, 6 insertions(+), 5 deletions(-)
> > 
> > diff --git a/sound/firewire/bebob/bebob_stream.c b/sound/firewire/bebob/bebob_stream.c
> > index f7f0db5aa811..1b264d0d63eb 100644
> > --- a/sound/firewire/bebob/bebob_stream.c
> > +++ b/sound/firewire/bebob/bebob_stream.c
> > @@ -415,15 +415,16 @@ static int make_both_connections(struct snd_bebob *bebob)
> >  	return 0;
> >  }
> >  
> > -static void
> > -break_both_connections(struct snd_bebob *bebob)
> > +static void break_both_connections(struct snd_bebob *bebob)
> >  {
> >  	cmp_connection_break(&bebob->in_conn);
> >  	cmp_connection_break(&bebob->out_conn);
> >  
> > -	/* These models seems to be in transition state for a longer time. */
> > -	if (bebob->maudio_special_quirk != NULL)
> > -		msleep(200);
> > +	// These models seem to be in transition state for a longer time. When
> > +	// accessing in the state, any transactions is corrupted. In the worst
> > +	// case, the device is going to reboot.
> > +	if (bebob->version < 2)
> > +		msleep(600);
> >  }
> >  
> >  static int
> > -- 
> > 2.20.1
> > 
> > _______________________________________________
> > Alsa-devel mailing list
> > Alsa-devel@alsa-project.org
> > https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> 
> This patch causes a reboot when I power on my Edirol FA-101. This is
> consistent behavior with kernels 5.5.1 - 5.5.11.
> 
> I've changed msleep(600) back to msleep(200) and rebuilt the module and
> I am once again able to power on and use the interface.
> 
> I am running Arch linux with 5.5.11-arch1-1 #1 SMP PREEMPT Sun, 22 Mar 2020 16:33:15 +0000 x86_64 GNU/Linux.
> 
> 
> I had previously posted to the list on Feb 19, 2020 with the subject:
> 
> sytem reboots when initializing Edirol FA-101 firewire audio interface
> 
> Please forgive any lapses in reporting protocol as I generally don't
> work at the kernel level. Hopefully this more specific report will help
> fix the issue.

This is weird.  If just changing the msleep value from 600 to 200
"fixes" the problem, it means that something is already fundamentally
broken...


Takashi

Re: [alsa-devel] [PATCH 1/3] ALSA: bebob: expand sleep just after breaking connections for protocol version 1

[Steve Morris]

On Fri, Mar 27, 2020 at 09:40:34AM +0100, Takashi Iwai wrote:
> On Thu, 26 Mar 2020 22:47:22 +0100,
> Steve Morris wrote:
> > 
> > On Fri, Nov 01, 2019 at 10:13:21PM +0900, Takashi Sakamoto wrote:
> > > As long as I investigated, some devices with BeBoB protocol version 1
> > > can be freezed during several hundreds milliseconds after breaking
> > > connections. When accessing during the freezed time, any transaction
> > > is corrupted. In the worst case, the device is going to reboot.
> > > 
> > > I can see this issue in:
> > >  * Roland FA-66
> > >  * M-Audio FireWire Solo
> > > 
> > > This commit expands sleep just after breaking connections to avoid
> > > the freezed time as much as possible. I note that the freeze/reboot
> > > behaviour is similar to below models:
> > >  * Focusrite Saffire Pro 10 I/O
> > >  * Focusrite Saffire Pro 26 I/O
> > > 
> > > The above models certainly reboot after breaking connections.
> > > 
> > > Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
> > > ---
> > >  sound/firewire/bebob/bebob_stream.c | 11 ++++++-----
> > >  1 file changed, 6 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/sound/firewire/bebob/bebob_stream.c b/sound/firewire/bebob/bebob_stream.c
> > > index f7f0db5aa811..1b264d0d63eb 100644
> > > --- a/sound/firewire/bebob/bebob_stream.c
> > > +++ b/sound/firewire/bebob/bebob_stream.c
> > > @@ -415,15 +415,16 @@ static int make_both_connections(struct snd_bebob *bebob)
> > >  	return 0;
> > >  }
> > >  
> > > -static void
> > > -break_both_connections(struct snd_bebob *bebob)
> > > +static void break_both_connections(struct snd_bebob *bebob)
> > >  {
> > >  	cmp_connection_break(&bebob->in_conn);
> > >  	cmp_connection_break(&bebob->out_conn);
> > >  
> > > -	/* These models seems to be in transition state for a longer time. */
> > > -	if (bebob->maudio_special_quirk != NULL)
> > > -		msleep(200);
> > > +	// These models seem to be in transition state for a longer time. When
> > > +	// accessing in the state, any transactions is corrupted. In the worst
> > > +	// case, the device is going to reboot.
> > > +	if (bebob->version < 2)
> > > +		msleep(600);
> > >  }
> > >  
> > >  static int
> > > -- 
> > > 2.20.1
> > > 
> > > _______________________________________________
> > > Alsa-devel mailing list
> > > Alsa-devel@alsa-project.org
> > > https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> > 
> > This patch causes a reboot when I power on my Edirol FA-101. This is
> > consistent behavior with kernels 5.5.1 - 5.5.11.
> > 
> > I've changed msleep(600) back to msleep(200) and rebuilt the module and
> > I am once again able to power on and use the interface.
> > 
> > I am running Arch linux with 5.5.11-arch1-1 #1 SMP PREEMPT Sun, 22 Mar 2020 16:33:15 +0000 x86_64 GNU/Linux.
> > 
> > 
> > I had previously posted to the list on Feb 19, 2020 with the subject:
> > 
> > sytem reboots when initializing Edirol FA-101 firewire audio interface
> > 
> > Please forgive any lapses in reporting protocol as I generally don't
> > work at the kernel level. Hopefully this more specific report will help
> > fix the issue.
> 
> This is weird.  If just changing the msleep value from 600 to 200
> "fixes" the problem, it means that something is already fundamentally
> broken...
> 
> 
> Takashi

It appears that 'fixed' was too strong a word. While I have managed to
get it to work a couple of times, generally it is still broken.

I'm happy to help debug, but this is outside my scope.

Back to the LTS kernel for the moment.

Steve

[alsa-devel] [PATCH 2/3] ALSA: bebob: expand delay of start for IR context just for version 3 firmware

[Takashi Sakamoto]

As long as I investigated, there's some cases about the delay for device
between establishing OUTPUT_PLUG and transmitting first isoc packet. For
devices which support BeBoB protocol version 1 can transmit the packet
within several hundred milliseconds, while for devices which support
BeBoB protocol version 3 can transmit the packet within 2 seconds.

Devices with protocol version 1:
 * Edirol FA-66
 * Yamaha GO46
 * Terratec Phase x24 FW
 * M-Audio FireWire AudioPhile
 * M-Audio FireWire Solo
 * M-Audio FireWire 1814
 * M-Audio FireWire 410
 * Focusrite Saffire Pro 26 I/O

Devices with protocol version 3:
 * M-Audio Profire Lightbridge
 * Behringer FCA610
 * Phonic Firefly 202

At present ALSA bebob driver postpones starting IR context during
1.5 sec for all of supported devices. The delay is too long for
devices with protocol version 1, while it's not enough for devices with
protocol version 3.

This commit improves the delay for these protocols.

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

diff --git a/sound/firewire/bebob/bebob_stream.c b/sound/firewire/bebob/bebob_stream.c
index 1b264d0d63eb..895a113635af 100644
--- a/sound/firewire/bebob/bebob_stream.c
+++ b/sound/firewire/bebob/bebob_stream.c
@@ -7,7 +7,7 @@
 
 #include "./bebob.h"
 
-#define CALLBACK_TIMEOUT	2000
+#define CALLBACK_TIMEOUT	2500
 #define FW_ISO_RESOURCE_DELAY	1000
 
 /*
@@ -639,6 +639,7 @@ int snd_bebob_stream_start_duplex(struct snd_bebob *bebob)
 
 	if (!amdtp_stream_running(&bebob->rx_stream)) {
 		unsigned int curr_rate;
+		unsigned int ir_delay_cycle;
 
 		if (bebob->maudio_special_quirk) {
 			err = bebob->spec->rate->get(bebob, &curr_rate);
@@ -660,13 +661,18 @@ int snd_bebob_stream_start_duplex(struct snd_bebob *bebob)
 
 		// The device postpones start of transmission mostly for 1 sec
 		// after receives packets firstly. For safe, IR context starts
-		// 1.5 sec (=12000 cycles) later. This is within 2.0 sec
-		// (=CALLBACK_TIMEOUT).
+		// 0.4 sec (=3200 cycles) later to version 1 or 2 firmware,
+		// 2.0 sec (=16000 cycles) for version 3 firmware. This is
+		// within 2.5 sec (=CALLBACK_TIMEOUT).
 		// Furthermore, some devices transfer isoc packets with
 		// discontinuous counter in the beginning of packet streaming.
 		// The delay has an effect to avoid detection of this
 		// discontinuity.
-		err = amdtp_domain_start(&bebob->domain, 12000);
+		if (bebob->version < 2)
+			ir_delay_cycle = 3200;
+		else
+			ir_delay_cycle = 16000;
+		err = amdtp_domain_start(&bebob->domain, ir_delay_cycle);
 		if (err < 0)
 			goto error;
 
-- 
2.20.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[alsa-devel] [PATCH 3/3] ALSA: bebob: link the order of establishing connections and Syt-match clock mode

[Takashi Sakamoto]

Originally BeBeB ASICs and firmware supports clock mode to synchronizing
to syt field of received isoc packet. This mode is known as 'SYT Match'
slightly described in IEC 61883-6 (but no detail mechanisms). In this
mode, drivers can control sampling clock in device. Driver for Windows
and macOS uses this feature to perform synchronization for devices
on the same bus.

In this mode, a plug of Music subunit for synchronization is connected
to a plug of isoc unit for incoming packet streaming, then the order to
establish connections is INPUT_PLUG first, OUTPUT_PLUG second.

This commit implements the above.

Actually each device works with its own clock for sampling, therefore
the original design is hardly implemented to vendor's products.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/bebob/bebob_stream.c | 51 +++++++++++++----------------
 1 file changed, 22 insertions(+), 29 deletions(-)

diff --git a/sound/firewire/bebob/bebob_stream.c b/sound/firewire/bebob/bebob_stream.c
index 895a113635af..bbae04793c50 100644
--- a/sound/firewire/bebob/bebob_stream.c
+++ b/sound/firewire/bebob/bebob_stream.c
@@ -398,23 +398,6 @@ check_connection_used_by_others(struct snd_bebob *bebob, struct amdtp_stream *s)
 	return err;
 }
 
-static int make_both_connections(struct snd_bebob *bebob)
-{
-	int err = 0;
-
-	err = cmp_connection_establish(&bebob->out_conn);
-	if (err < 0)
-		return err;
-
-	err = cmp_connection_establish(&bebob->in_conn);
-	if (err < 0) {
-		cmp_connection_break(&bebob->out_conn);
-		return err;
-	}
-
-	return 0;
-}
-
 static void break_both_connections(struct snd_bebob *bebob)
 {
 	cmp_connection_break(&bebob->in_conn);
@@ -427,8 +410,7 @@ static void break_both_connections(struct snd_bebob *bebob)
 		msleep(600);
 }
 
-static int
-start_stream(struct snd_bebob *bebob, struct amdtp_stream *stream)
+static int start_stream(struct snd_bebob *bebob, struct amdtp_stream *stream)
 {
 	struct cmp_connection *conn;
 	int err = 0;
@@ -438,18 +420,19 @@ start_stream(struct snd_bebob *bebob, struct amdtp_stream *stream)
 	else
 		conn = &bebob->out_conn;
 
-	/* channel mapping */
+	// channel mapping.
 	if (bebob->maudio_special_quirk == NULL) {
 		err = map_data_channels(bebob, stream);
 		if (err < 0)
-			goto end;
+			return err;
 	}
 
-	// start amdtp stream.
-	err = amdtp_domain_add_stream(&bebob->domain, stream,
-				      conn->resources.channel, conn->speed);
-end:
-	return err;
+	err = cmp_connection_establish(conn);
+	if (err < 0)
+		return err;
+
+	return amdtp_domain_add_stream(&bebob->domain, stream,
+				       conn->resources.channel, conn->speed);
 }
 
 static int init_stream(struct snd_bebob *bebob, struct amdtp_stream *stream)
@@ -638,6 +621,8 @@ int snd_bebob_stream_start_duplex(struct snd_bebob *bebob)
 	}
 
 	if (!amdtp_stream_running(&bebob->rx_stream)) {
+		enum snd_bebob_clock_type src;
+		struct amdtp_stream *master, *slave;
 		unsigned int curr_rate;
 		unsigned int ir_delay_cycle;
 
@@ -647,15 +632,23 @@ int snd_bebob_stream_start_duplex(struct snd_bebob *bebob)
 				return err;
 		}
 
-		err = make_both_connections(bebob);
+		err = snd_bebob_stream_get_clock_src(bebob, &src);
 		if (err < 0)
 			return err;
 
-		err = start_stream(bebob, &bebob->rx_stream);
+		if (src != SND_BEBOB_CLOCK_TYPE_SYT) {
+			master = &bebob->tx_stream;
+			slave = &bebob->rx_stream;
+		} else {
+			master = &bebob->rx_stream;
+			slave = &bebob->tx_stream;
+		}
+
+		err = start_stream(bebob, master);
 		if (err < 0)
 			goto error;
 
-		err = start_stream(bebob, &bebob->tx_stream);
+		err = start_stream(bebob, slave);
 		if (err < 0)
 			goto error;
 
-- 
2.20.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] [PATCH 0/3] ALSA: bebob: misc improvements for starting/stopping packet streaming

[Takashi Iwai]

On Fri, 01 Nov 2019 14:13:20 +0100,
Takashi Sakamoto wrote:
> 
> Hi,
> 
> This patchset improves model-specific issues in a point of protocol
> version supported by the model. The changes are relevant to packet
> streaming, thus it can brings some regressions.
> 
> For testers, I prepare for a remote branch to backport the latest
> codes to Linux v4.17 or later. I'm happy to receive any report.
> https://github.com/takaswie/snd-firewire-improve/tree/topic/bebob-connection-issue
> 
> Takashi Sakamoto (3):
>   ALSA: bebob: expand sleep just after breaking connections for protocol
>     version 1
>   ALSA: bebob: expand delay of start for IR context just for version 3
>     firmware
>   ALSA: bebob: link the order of establishing connections and Syt-match
>     clock mode

Applies all patches now.


thanks,

Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel