[PATCH 0/3] ALSA: firewire: media clock recovery for syt-aware devices

[PATCH 0/3] ALSA: firewire: media clock recovery for syt-aware devices

[Takashi Sakamoto]

Hi,

In a commit f9e5ecdfc2c2 ("ALSA: firewire-lib: add replay target to cache
sequence of packet"), I categorize devices supported by drivers in ALSA
firewire stack in terms of the way to deliver effective sampling
transfer frequency. This patchset is for the devices in group 2.

The devices are known to have problems when ALSA dice/bebob drivers
handle. Many of them sometimes transfer packets with discontinued counter,
corrupt at break of CMP connection, generates bus-reset voluntarily.

The devices interpret presentation time to decide playback timing. The
drivers process presentation time expressed in syt field of CIP header for
outgoing packets. Current implementation of the drivers processes the
sequence of outgoing packet by computation according to nominal sampling
transfer frequency, assisted by ALSA IEC 61883-1/6 packet streaming engine.
However, the ideal sequence is not adequate to the devices, actually.

With this patchset, the drivers are going to replay the sequence of
incoming packets for media clock recovery. For the detail of sequence
replay, please refer to a commit 39c2649c71d8 ("ALSA: firewire-lib: replay
sequence of incoming packets for outgoing packets").

Takashi Sakamoto (3):
  ALSA: dice: wait just for NOTIFY_CLOCK_ACCEPTED after
    GLOBAL_CLOCK_SELECT operation
  ALSA: dice: perform sequence replay for media clock recovery
  ALSA: bebob: perform sequence replay for media clock recovery

 sound/firewire/bebob/bebob_stream.c    | 13 +++++++---
 sound/firewire/dice/dice-stream.c      | 35 +++++++++-----------------
 sound/firewire/dice/dice-transaction.c |  2 +-
 3 files changed, 22 insertions(+), 28 deletions(-)

-- 
2.27.0

[PATCH 1/3] ALSA: dice: wait just for NOTIFY_CLOCK_ACCEPTED after GLOBAL_CLOCK_SELECT operation

[Takashi Sakamoto]

NOTIFY_CLOCK_ACCEPTED notification is always generated as a result of
GLOBAL_CLOCK_SELECT operation, however NOTIFY_LOCK_CHG notification
doesn't, as long as the selected clock is already configured. In the case,
ALSA dice driver waits so long. It's inconvenient for some devices to lock
to the sequence of value in syt field of CIP header in rx packets.

This commit wait just for NOTIFY_CLOCK_ACCEPTED notification by reverting
changes partially done by two commits below:

 * commit fbeac84dbe9e ("ALSA: dice: old firmware optimization for Dice notification")
 * commit aec045b80d79 ("ALSA: dice: change notification mask to detect lock status change")

I note that the successful lock to the sequence of value in syt field of
CIP header in rx packets results in NOTIFY_EXT_STATUS notification, then
EXT_STATUS_ARX1_LOCKED bit stands in GLOBAL_EXTENDED_STATUS register.
The notification can occur enough after receiving the batch of rx packets.
When the sequence doesn't include value in syt field of CIP header in rx
packets adequate to the device, the notification occurs again and the bit
is off.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/dice/dice-stream.c      | 29 +++++++-------------------
 sound/firewire/dice/dice-transaction.c |  2 +-
 2 files changed, 8 insertions(+), 23 deletions(-)

diff --git a/sound/firewire/dice/dice-stream.c b/sound/firewire/dice/dice-stream.c
index 0fb8b4ae6a0a..d7220160c778 100644
--- a/sound/firewire/dice/dice-stream.c
+++ b/sound/firewire/dice/dice-stream.c
@@ -9,7 +9,7 @@
 #include "dice.h"
 
 #define	READY_TIMEOUT_MS	200
-#define NOTIFICATION_TIMEOUT_MS	(2 * MSEC_PER_SEC)
+#define NOTIFICATION_TIMEOUT_MS	100
 
 struct reg_params {
 	unsigned int count;
@@ -57,13 +57,9 @@ int snd_dice_stream_get_rate_mode(struct snd_dice *dice, unsigned int rate,
 	return -EINVAL;
 }
 
-/*
- * This operation has an effect to synchronize GLOBAL_STATUS/GLOBAL_SAMPLE_RATE
- * to GLOBAL_STATUS. Especially, just after powering on, these are different.
- */
-static int ensure_phase_lock(struct snd_dice *dice, unsigned int rate)
+static int select_clock(struct snd_dice *dice, unsigned int rate)
 {
-	__be32 reg, nominal;
+	__be32 reg;
 	u32 data;
 	int i;
 	int err;
@@ -94,19 +90,8 @@ static int ensure_phase_lock(struct snd_dice *dice, unsigned int rate)
 		return err;
 
 	if (wait_for_completion_timeout(&dice->clock_accepted,
-			msecs_to_jiffies(NOTIFICATION_TIMEOUT_MS)) == 0) {
-		/*
-		 * Old versions of Dice firmware transfer no notification when
-		 * the same clock status as current one is set. In this case,
-		 * just check current clock status.
-		 */
-		err = snd_dice_transaction_read_global(dice, GLOBAL_STATUS,
-						&nominal, sizeof(nominal));
-		if (err < 0)
-			return err;
-		if (!(be32_to_cpu(nominal) & STATUS_SOURCE_LOCKED))
-			return -ETIMEDOUT;
-	}
+			msecs_to_jiffies(NOTIFICATION_TIMEOUT_MS)) == 0)
+		return -ETIMEDOUT;
 
 	return 0;
 }
@@ -304,7 +289,7 @@ int snd_dice_stream_reserve_duplex(struct snd_dice *dice, unsigned int rate,
 		// 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);
+		err = select_clock(dice, rate);
 		if (err < 0)
 			return err;
 
@@ -646,7 +631,7 @@ int snd_dice_stream_detect_current_formats(struct snd_dice *dice)
 	 * invalid stream formats. Selecting clock parameters have an effect
 	 * for the unit to refine it.
 	 */
-	err = ensure_phase_lock(dice, rate);
+	err = select_clock(dice, rate);
 	if (err < 0)
 		return err;
 
diff --git a/sound/firewire/dice/dice-transaction.c b/sound/firewire/dice/dice-transaction.c
index 2c0dde29a024..92941ef83cd5 100644
--- a/sound/firewire/dice/dice-transaction.c
+++ b/sound/firewire/dice/dice-transaction.c
@@ -155,7 +155,7 @@ static void dice_notification(struct fw_card *card, struct fw_request *request,
 
 	fw_send_response(card, request, RCODE_COMPLETE);
 
-	if (bits & NOTIFY_LOCK_CHG)
+	if (bits & NOTIFY_CLOCK_ACCEPTED)
 		complete(&dice->clock_accepted);
 	wake_up(&dice->hwdep_wait);
 }
-- 
2.27.0

[PATCH 2/3] ALSA: dice: perform sequence replay for media clock recovery

[Takashi Sakamoto]

This commit takes ALSA dice driver to perform sequence replay for media
clock recovery.

Unlike the other types of device, DICE-based devices interpret the value
of syt field of CIP header in rx packets as presentation time for audio
playback, thus it's required for driver to compute value for outgoing
packet adequate to the device. It's done by media clock recovery by
handling tx packets.

The device starts packet transmission immediately at operation to
GLOBAL_ENABLE thus on-the-fly mode is not required.

DICE ASICs supports several pairs of isochronous packet streams.
Actually, maximum two pairs of streams are supported by devices.
We have three cases regarding to the number of streams:

1. a pair of streams
2. two tx packet streams and one rx packet streams
3. one tx packet streams and two rx packet streams
4. two pair of streams

The decision of playback timing is slightly different in the four cases.

In the case 1, sequence replay in the pair results in suitable playback
timing.

In the case 2, sequence replay from the first tx packet stream to rx
packet stream results in suitable playback timing.

In the case 3, sequence replay from tx packet stream to all of rx packet
stream results in suitable playback timing. Furthermore, the cycle to
start receiving packets should be the same between all rx packet streams.

In the case 4, sequence replay in each pair results in suitable playback
timing. Furthermore, the cycle to start receiving packets should be the
same between all rx packet streams.

The sequence replay is tested with below models:

* For case 1:
  * TC Electronic Konnekt 24d (DiceII)
  * TC Electronic Konnekt 8 (DiceII)
  * TC Electronic Konnekt Live (DiceII)
  * TC Electronic Impact Twin (DiceII)
  * TC Electronic Digital Konnekt X32 (DiceII)
  * TC Electronic Desktop Konnekt 6 (TCD2220)
  * Solid State Logic Duende Classic (DiceII)
  * Solid State Logic Duende Mini (DiceII)
  * PreSonus FireStudio Project (TCD2210)
  * PreSonus FireStudio Mobile (TCD2210)
  * Lexicon I-ONIX FW810s (TCD2220)
  * Avid Mbox 3 Pro (TCD2220)

* For case 2 (but case 1 depends on sampling transfer frequency):
  * Alesis iO 26 (DiceII)
  * Alesis iO 14 (DiceII)
  * Alesis MultiMix 12 FireWire (DiceII)
  * Focusrite Saffire Pro 26 (TCD2220)

* For case 3 (but case 1 depends on sampling transfer frequency):
  * M-Audio Profire 610 (TCD2220)
  * Loud Technology Mackie Onyx Blackbird (TCD2210)

* For case 4:
  * TC Electronic Studio Konnekt 48 (DiceII + TCD2220)
  * PreSonus FireStudio (DiceII)
  * M-Audio Profire 2626 (TCD2220)
  * Focusrite Liquid Saffire 56 (TCD2220)
  * Focusrite Saffire Pro 40 (TCD2220)

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

diff --git a/sound/firewire/dice/dice-stream.c b/sound/firewire/dice/dice-stream.c
index d7220160c778..f99e00083141 100644
--- a/sound/firewire/dice/dice-stream.c
+++ b/sound/firewire/dice/dice-stream.c
@@ -444,7 +444,11 @@ int snd_dice_stream_start_duplex(struct snd_dice *dice)
 			goto error;
 		}
 
-		err = amdtp_domain_start(&dice->domain, 0, false, false);
+		// MEMO: The device immediately starts packet transmission when enabled. Some
+		// devices are strictly to generate any discontinuity in the sequence of tx packet
+		// when they receives invalid sequence of presentation time in CIP header. The
+		// sequence replay for media clock recovery can suppress the behaviour.
+		err = amdtp_domain_start(&dice->domain, 0, true, false);
 		if (err < 0)
 			goto error;
 
-- 
2.27.0

[PATCH 3/3] ALSA: bebob: perform sequence replay for media clock recovery

[Takashi Sakamoto]

This commit takes ALSA bebob driver to perform sequence replay for media
clock recovery.

Many users have reported discontinuity of data block counter field of CIP
header in tx packet from the devices based on BeBoB ASICs. In the worst
case, the device corrupts not to respond to any transaction, then generate
bus-reset voluntarily for recovery. The sequence replay for media clock
recovery is expected to suppress most of the problems.

In the beginning of packet streaming, the device transfers NODATA packets
for a while, then multiplexes any event and syt information. ALSA
IEC 61883-1/6 packet streaming engine has implementation for it to drop
the initial NODATA packets. It starts sequence replay when detecting any
event multiplexed to tx packets.

The sequence replay is tested with below models:

 * Focusrite Saffire
 * Focusrite Saffire LE
 * Focusrite Saffire Pro 10 I/O
 * Focusrite Saffire Pro 26 I/O
 * M-Audio FireWire Solo
 * M-Audio FireWire Audiophile
 * M-Audio Ozonic
 * M-Audio FireWire 410
 * M-Audio FireWire 1814
 * Edirol FA-66
 * ESI Quatafire 610
 * Apogee Ensemble
 * Phonic Firefly 202
 * Behringer F-Control Audio 610

Unfortunately, below models doesn't generate sound. This seems regression
introduced recent few years:

 * Stanton Final Scratch ScratchAmp at middle sampling transfer frequency
 * Yamaha GO44
 * Yamaha GO46
 * Terratec Phase x24

As I reported, below model has quirk of discontinuity:

 * M-Audio ProFire Lightbridge

DM1000/DM1100 ASICs in BeBoB solution are known to have bugs at switch of
sampling transfer frequency between low/middle/high rates. The switch
generates the similar problems about which I mention in the above. Some
vendors customizes firmware so that the switch of frequency is done in
vendor-specific registers, then restrict users to switch the frequency.

For example of Focusrite Saffire Pro 10 i/o and 26 i/o, users allows to
switch the frequency within the three steps; e.g. 44.1/48.0 kHz are
available at low step. Between the steps, extra operation is required and
it always generates bus-reset.

Another example of Edirol FA-66, users are prohibited to switch the
frequency by software. It's done by hardware switch and power-off.

I note that the sequence replay is not a solution for the ASIC bugs. Users
need to disconnect the device corrupted by the bug, then reconnect it to
refresh state machine inner the ASIC.

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

diff --git a/sound/firewire/bebob/bebob_stream.c b/sound/firewire/bebob/bebob_stream.c
index 47773ca97e46..470c2b70cbfa 100644
--- a/sound/firewire/bebob/bebob_stream.c
+++ b/sound/firewire/bebob/bebob_stream.c
@@ -649,10 +649,15 @@ int snd_bebob_stream_start_duplex(struct snd_bebob *bebob)
 		else
 			tx_init_skip_cycles = 16000;
 
-		// MEMO: In the early stage of packet streaming, the device transfers NODATA packets.
-		// After several hundred cycles, it begins to multiplex event into the packet with
-		// syt information.
-		err = amdtp_domain_start(&bebob->domain, tx_init_skip_cycles, false, false);
+		// MEMO: Some devices start packet transmission long enough after establishment of
+		// CMP connection. In the early stage of packet streaming, any device transfers
+		// NODATA packets. After several hundred cycles, it begins to multiplex event into
+		// the packet with adequate value of syt field in CIP header. Some devices are
+		// strictly to generate any discontinuity in the sequence of tx packet when they
+		// receives inadequate sequence of value in syt field of CIP header. In the case,
+		// the request to break CMP connection is often corrupted, then any transaction
+		// results in unrecoverable error, sometimes generate bus-reset.
+		err = amdtp_domain_start(&bebob->domain, tx_init_skip_cycles, true, false);
 		if (err < 0)
 			goto error;
 
-- 
2.27.0

Re: [PATCH 0/3] ALSA: firewire: media clock recovery for syt-aware devices

[Takashi Iwai]

On Tue, 01 Jun 2021 10:17:50 +0200,
Takashi Sakamoto wrote:
> 
> Hi,
> 
> In a commit f9e5ecdfc2c2 ("ALSA: firewire-lib: add replay target to cache
> sequence of packet"), I categorize devices supported by drivers in ALSA
> firewire stack in terms of the way to deliver effective sampling
> transfer frequency. This patchset is for the devices in group 2.
> 
> The devices are known to have problems when ALSA dice/bebob drivers
> handle. Many of them sometimes transfer packets with discontinued counter,
> corrupt at break of CMP connection, generates bus-reset voluntarily.
> 
> The devices interpret presentation time to decide playback timing. The
> drivers process presentation time expressed in syt field of CIP header for
> outgoing packets. Current implementation of the drivers processes the
> sequence of outgoing packet by computation according to nominal sampling
> transfer frequency, assisted by ALSA IEC 61883-1/6 packet streaming engine.
> However, the ideal sequence is not adequate to the devices, actually.
> 
> With this patchset, the drivers are going to replay the sequence of
> incoming packets for media clock recovery. For the detail of sequence
> replay, please refer to a commit 39c2649c71d8 ("ALSA: firewire-lib: replay
> sequence of incoming packets for outgoing packets").
> 
> Takashi Sakamoto (3):
>   ALSA: dice: wait just for NOTIFY_CLOCK_ACCEPTED after
>     GLOBAL_CLOCK_SELECT operation
>   ALSA: dice: perform sequence replay for media clock recovery
>   ALSA: bebob: perform sequence replay for media clock recovery

Applied all three patches now.  Thanks.


Takashi

Re: [PATCH 2/3] ALSA: dice: perform sequence replay for media clock recovery

[Hector Martin]

On 01/06/2021 17.17, Takashi Sakamoto wrote:
> This commit takes ALSA dice driver to perform sequence replay for media
> clock recovery.

Just wanted to report back that I've been running tiwai/for-next 
including this patch and others for about a week now, and everything 
works great on a Focusrite Saffire 26; it's more stable than ffado with 
JACK and PipeWire also works. Thank you for finally fixing this!

One thing: I've noticed that on these interfaces, each 
transmitter/receiver gets a different PCM device (e.g. on this one, two 
capture PCM devices and one playback PCM device). I assume that 
bonding/simultaneous use is left for userspace to do, and that it can be 
done sample-accurately, right? This might be a little annoying for JACK, 
though not an issue for PipeWire as far as I know. With libffado on 
these DICE devices, the library takes care of bonding all the tx/rx 
groups into one set of channels.

Tested-by: Hector Martin <marcan@marcan.st>

-- 
Hector Martin (marcan@marcan.st)
Public Key: https://mrcn.st/pub