[PATCH 0/3] ALSA: firewire-motu: media clock recovery for sph-aware devices
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 3.
The devices are known to have problems when ALSA firewire-motu driver handles. Many of them generate sound with noise. In the worst case, it generates no sound.
The devices interpret presentation time to decide playback timing. Unlike the syt-aware devices, the devices interpret the presentation time in source packet header (SPH) per data block, instead of the presentation time in syt field of CIP header.
Current implementation of the driver processes the sequence of outgoing packet by computation according to nominal sampling transfer frequency. 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, instead of nominal sampling transfer frequency. For the detail of sequence replay, please refer to a commit 39c2649c71d8 ("ALSA: firewire-lib: replay sequence of incoming packets for outgoing packets"). The sequence replay is done by two levels; the sequence of the number of data blocks per packet, and the sequence of SPH per data blocks in the packet.
Takashi Sakamoto (3): ALSA: firewire-motu: use macro for magic numbers relevant to IEC 61883-1 ALSA: firewire-motu: cache event ticks in source packet header per data block ALSA: firewire-motu: sequence replay for source packet header
sound/firewire/motu/amdtp-motu.c | 136 +++++++++++++++--------------- sound/firewire/motu/motu-stream.c | 27 +++++- sound/firewire/motu/motu.h | 14 ++- 3 files changed, 104 insertions(+), 73 deletions(-)
ALSA firewire-motu driver has some magic numbers from IEC 61883-1 to operates source packet header (SPH). This commit replaces them with macros.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/firewire/motu/amdtp-motu.c | 40 +++++++++++++++++++++----------- 1 file changed, 26 insertions(+), 14 deletions(-)
diff --git a/sound/firewire/motu/amdtp-motu.c b/sound/firewire/motu/amdtp-motu.c index 18bf433f43b6..89638e1fbb69 100644 --- a/sound/firewire/motu/amdtp-motu.c +++ b/sound/firewire/motu/amdtp-motu.c @@ -16,6 +16,18 @@ #define CIP_FMT_MOTU_TX_V3 0x22 #define MOTU_FDF_AM824 0x22
+#define TICKS_PER_CYCLE 3072 +#define CYCLES_PER_SECOND 8000 +#define TICKS_PER_SECOND (TICKS_PER_CYCLE * CYCLES_PER_SECOND) + +#define IEEE1394_SEC_MODULUS 128 + +#define TRANSFER_DELAY_TICKS 0x2e00 /* 479.17 microseconds */ + +#define CIP_SPH_CYCLE_SHIFT 12 +#define CIP_SPH_CYCLE_MASK 0x01fff000 +#define CIP_SPH_OFFSET_MASK 0x00000fff + /* * Nominally 3125 bytes/second, but the MIDI port's clock might be * 1% too slow, and the bus clock 100 ppm too fast. @@ -97,17 +109,16 @@ int amdtp_motu_set_parameters(struct amdtp_stream *s, unsigned int rate, p->midi_db_count = 0; p->midi_db_interval = rate / MIDI_BYTES_PER_SECOND;
- /* IEEE 1394 bus requires. */ - delay = 0x2e00; + delay = TRANSFER_DELAY_TICKS;
- /* For no-data or empty packets to adjust PCM sampling frequency. */ - delay += 8000 * 3072 * s->syt_interval / rate; + // For no-data or empty packets to adjust PCM sampling frequency. + delay += TICKS_PER_SECOND * s->syt_interval / rate;
p->next_seconds = 0; - p->next_cycles = delay / 3072; + p->next_cycles = delay / TICKS_PER_CYCLE; p->quotient_ticks_per_event = params[s->sfc].quotient_ticks_per_event; p->remainder_ticks_per_event = params[s->sfc].remainder_ticks_per_event; - p->next_ticks = delay % 3072; + p->next_ticks = delay % TICKS_PER_CYCLE; p->next_accumulated = 0;
return 0; @@ -363,18 +374,18 @@ static inline void compute_next_elapse_from_start(struct amdtp_motu *p) }
p->next_ticks += p->quotient_ticks_per_event; - if (p->next_ticks >= 3072) { - p->next_ticks -= 3072; + if (p->next_ticks >= TICKS_PER_CYCLE) { + p->next_ticks -= TICKS_PER_CYCLE; p->next_cycles++; }
- if (p->next_cycles >= 8000) { - p->next_cycles -= 8000; + if (p->next_cycles >= CYCLES_PER_SECOND) { + p->next_cycles -= CYCLES_PER_SECOND; p->next_seconds++; }
- if (p->next_seconds >= 128) - p->next_seconds -= 128; + if (p->next_seconds >= IEEE1394_SEC_MODULUS) + p->next_seconds -= IEEE1394_SEC_MODULUS; }
static void write_sph(struct amdtp_stream *s, __be32 *buffer, unsigned int data_blocks, @@ -386,8 +397,9 @@ static void write_sph(struct amdtp_stream *s, __be32 *buffer, unsigned int data_ u32 sph;
for (i = 0; i < data_blocks; i++) { - next_cycles = (rx_start_cycle + p->next_cycles) % 8000; - sph = ((next_cycles << 12) | p->next_ticks) & 0x01ffffff; + next_cycles = (rx_start_cycle + p->next_cycles) % CYCLES_PER_SECOND; + sph = ((next_cycles << CIP_SPH_CYCLE_SHIFT) | p->next_ticks) & + (CIP_SPH_CYCLE_MASK | CIP_SPH_OFFSET_MASK); *buffer = cpu_to_be32(sph);
compute_next_elapse_from_start(p);
The devices in MOTU FireWire series put source packet header (SPH) into each data block of tx packet for presentation time of event. The format of timestamp is compliant to IEC 61883-1, with cycle and offset fields without sec field of 32 bit cycle time.
This commit takes ALSA firewire-motu driver to cache the presentation time as offset from cycle in which the packet is transferred.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/firewire/motu/amdtp-motu.c | 41 ++++++++++++++++++++++++++++++- sound/firewire/motu/motu-stream.c | 20 ++++++++++++++- sound/firewire/motu/motu.h | 12 ++++++++- 3 files changed, 70 insertions(+), 3 deletions(-)
diff --git a/sound/firewire/motu/amdtp-motu.c b/sound/firewire/motu/amdtp-motu.c index 89638e1fbb69..1741ceb381c7 100644 --- a/sound/firewire/motu/amdtp-motu.c +++ b/sound/firewire/motu/amdtp-motu.c @@ -53,6 +53,8 @@ struct amdtp_motu {
int midi_db_count; unsigned int midi_db_interval; + + struct amdtp_motu_cache *cache; };
int amdtp_motu_set_parameters(struct amdtp_stream *s, unsigned int rate, @@ -333,6 +335,34 @@ static void probe_tracepoints_events(struct amdtp_stream *s, } }
+static void cache_event_offsets(struct amdtp_motu_cache *cache, const __be32 *buf, + unsigned int data_blocks, unsigned int data_block_quadlets) +{ + unsigned int *event_offsets = cache->event_offsets; + const unsigned int cache_size = cache->size; + unsigned int cache_tail = cache->tail; + unsigned int base_tick = cache->tx_cycle_count * TICKS_PER_CYCLE; + int i; + + for (i = 0; i < data_blocks; ++i) { + u32 sph = be32_to_cpu(*buf); + unsigned int tick; + + tick = ((sph & CIP_SPH_CYCLE_MASK) >> CIP_SPH_CYCLE_SHIFT) * TICKS_PER_CYCLE + + (sph & CIP_SPH_OFFSET_MASK); + + if (tick < base_tick) + tick += TICKS_PER_SECOND; + event_offsets[cache_tail] = tick - base_tick; + + cache_tail = (cache_tail + 1) % cache_size; + buf += data_block_quadlets; + } + + cache->tail = cache_tail; + cache->tx_cycle_count = (cache->tx_cycle_count + 1) % CYCLES_PER_SECOND; +} + static unsigned int process_ir_ctx_payloads(struct amdtp_stream *s, const struct pkt_desc *descs, unsigned int packets, @@ -342,12 +372,17 @@ static unsigned int process_ir_ctx_payloads(struct amdtp_stream *s, unsigned int pcm_frames = 0; int i;
+ if (p->cache->tx_cycle_count == UINT_MAX) + p->cache->tx_cycle_count = (s->domain->processing_cycle.tx_start % CYCLES_PER_SECOND); + // For data block processing. for (i = 0; i < packets; ++i) { const struct pkt_desc *desc = descs + i; __be32 *buf = desc->ctx_payload; unsigned int data_blocks = desc->data_blocks;
+ cache_event_offsets(p->cache, buf, data_blocks, s->data_block_quadlets); + if (pcm) { read_pcm_s32(s, pcm, buf, data_blocks, pcm_frames); pcm_frames += data_blocks; @@ -449,11 +484,12 @@ static unsigned int process_it_ctx_payloads(struct amdtp_stream *s,
int amdtp_motu_init(struct amdtp_stream *s, struct fw_unit *unit, enum amdtp_stream_direction dir, - const struct snd_motu_spec *spec) + const struct snd_motu_spec *spec, struct amdtp_motu_cache *cache) { amdtp_stream_process_ctx_payloads_t process_ctx_payloads; int fmt = CIP_FMT_MOTU; unsigned int flags = CIP_BLOCKING | CIP_UNAWARE_SYT; + struct amdtp_motu *p; int err;
if (dir == AMDTP_IN_STREAM) { @@ -493,5 +529,8 @@ int amdtp_motu_init(struct amdtp_stream *s, struct fw_unit *unit, s->ctx_data.rx.fdf = MOTU_FDF_AM824; }
+ p = s->protocol; + p->cache = cache; + return 0; } diff --git a/sound/firewire/motu/motu-stream.c b/sound/firewire/motu/motu-stream.c index 5d8d067f366d..369002568b2d 100644 --- a/sound/firewire/motu/motu-stream.c +++ b/sound/firewire/motu/motu-stream.c @@ -153,6 +153,9 @@ int snd_motu_stream_reserve_duplex(struct snd_motu *motu, unsigned int rate, fw_iso_resources_free(&motu->tx_resources); fw_iso_resources_free(&motu->rx_resources);
+ kfree(motu->cache.event_offsets); + motu->cache.event_offsets = NULL; + err = snd_motu_protocol_set_clock_rate(motu, rate); if (err < 0) { dev_err(&motu->unit->device, @@ -181,6 +184,15 @@ int snd_motu_stream_reserve_duplex(struct snd_motu *motu, unsigned int rate, fw_iso_resources_free(&motu->rx_resources); return err; } + + motu->cache.size = motu->tx_stream.syt_interval * frames_per_buffer; + motu->cache.event_offsets = kcalloc(motu->cache.size, sizeof(*motu->cache.event_offsets), + GFP_KERNEL); + if (!motu->cache.event_offsets) { + fw_iso_resources_free(&motu->tx_resources); + fw_iso_resources_free(&motu->rx_resources); + return err; + } }
return 0; @@ -260,6 +272,9 @@ int snd_motu_stream_start_duplex(struct snd_motu *motu) if (err < 0) goto stop_streams;
+ motu->cache.tail = 0; + motu->cache.tx_cycle_count = UINT_MAX; + err = amdtp_domain_start(&motu->domain, 0, false, false); if (err < 0) goto stop_streams; @@ -293,6 +308,9 @@ void snd_motu_stream_stop_duplex(struct snd_motu *motu)
fw_iso_resources_free(&motu->tx_resources); fw_iso_resources_free(&motu->rx_resources); + + kfree(motu->cache.event_offsets); + motu->cache.event_offsets = NULL; } }
@@ -314,7 +332,7 @@ static int init_stream(struct snd_motu *motu, struct amdtp_stream *s) if (err < 0) return err;
- err = amdtp_motu_init(s, motu->unit, dir, motu->spec); + err = amdtp_motu_init(s, motu->unit, dir, motu->spec, &motu->cache); if (err < 0) fw_iso_resources_destroy(resources);
diff --git a/sound/firewire/motu/motu.h b/sound/firewire/motu/motu.h index 92effb6e6c96..10ba87062e81 100644 --- a/sound/firewire/motu/motu.h +++ b/sound/firewire/motu/motu.h @@ -39,6 +39,13 @@ struct snd_motu_packet_format { unsigned char pcm_chunks[3]; };
+struct amdtp_motu_cache { + unsigned int *event_offsets; + unsigned int size; + unsigned int tail; + unsigned int tx_cycle_count; +}; + struct snd_motu { struct snd_card *card; struct fw_unit *unit; @@ -70,6 +77,8 @@ struct snd_motu { wait_queue_head_t hwdep_wait;
struct amdtp_domain domain; + + struct amdtp_motu_cache cache; };
enum snd_motu_spec_flags { @@ -125,7 +134,8 @@ extern const struct snd_motu_spec snd_motu_spec_4pre;
int amdtp_motu_init(struct amdtp_stream *s, struct fw_unit *unit, enum amdtp_stream_direction dir, - const struct snd_motu_spec *spec); + const struct snd_motu_spec *spec, + struct amdtp_motu_cache *cache); int amdtp_motu_set_parameters(struct amdtp_stream *s, unsigned int rate, unsigned int midi_ports, struct snd_motu_packet_format *formats);
This commit takes ALSA firewire-motu driver to perform sequence replay for media clock recovery.
Unlike the other types of device, the devices in MOTU FireWire series require two levels of sequence replay; the sequence of the number of data blocks per packet and the sequence of source packet header per data block. The former is already cached by ALSA IEC 61883-1/6 packet streaming engine and ready to be replayed. The latter is also cached by ALSA firewire-motu driver itself with a previous patch. This commit takes the driver to replay both of them from the caches.
The sequence replay is tested with below models:
* 828 mkII * Traveler * UltraLite * 828 mk3 FireWire * 828 mk3 Hybrid (except for high sampling transfer frequency * UltraLite mk3 FireWire * 4pre * AudioExpress
Unfortunately, below models still don't generate better sound, requires more work:
* 8pre * 828 mk3 Hybrid at high sampling transfer frequency
As long as I know, MOTU protocol version 1 requires extra care of the format of data block, thus below models are not supported yet in this time:
* 828 * 896
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/firewire/motu/amdtp-motu.c | 91 ++++++------------------------- sound/firewire/motu/motu-stream.c | 7 ++- sound/firewire/motu/motu.h | 2 + 3 files changed, 26 insertions(+), 74 deletions(-)
diff --git a/sound/firewire/motu/amdtp-motu.c b/sound/firewire/motu/amdtp-motu.c index 1741ceb381c7..5388b85fb60e 100644 --- a/sound/firewire/motu/amdtp-motu.c +++ b/sound/firewire/motu/amdtp-motu.c @@ -20,10 +20,6 @@ #define CYCLES_PER_SECOND 8000 #define TICKS_PER_SECOND (TICKS_PER_CYCLE * CYCLES_PER_SECOND)
-#define IEEE1394_SEC_MODULUS 128 - -#define TRANSFER_DELAY_TICKS 0x2e00 /* 479.17 microseconds */ - #define CIP_SPH_CYCLE_SHIFT 12 #define CIP_SPH_CYCLE_MASK 0x01fff000 #define CIP_SPH_OFFSET_MASK 0x00000fff @@ -35,14 +31,6 @@ #define MIDI_BYTES_PER_SECOND 3093
struct amdtp_motu { - /* For timestamp processing. */ - unsigned int quotient_ticks_per_event; - unsigned int remainder_ticks_per_event; - unsigned int next_ticks; - unsigned int next_accumulated; - unsigned int next_cycles; - unsigned int next_seconds; - unsigned int pcm_chunks; unsigned int pcm_byte_offset;
@@ -61,20 +49,8 @@ int amdtp_motu_set_parameters(struct amdtp_stream *s, unsigned int rate, unsigned int midi_ports, struct snd_motu_packet_format *formats) { - static const struct { - unsigned int quotient_ticks_per_event; - unsigned int remainder_ticks_per_event; - } params[] = { - [CIP_SFC_44100] = { 557, 123 }, - [CIP_SFC_48000] = { 512, 0 }, - [CIP_SFC_88200] = { 278, 282 }, - [CIP_SFC_96000] = { 256, 0 }, - [CIP_SFC_176400] = { 139, 141 }, - [CIP_SFC_192000] = { 128, 0 }, - }; struct amdtp_motu *p = s->protocol; unsigned int pcm_chunks, data_chunks, data_block_quadlets; - unsigned int delay; unsigned int mode; int i, err;
@@ -111,18 +87,6 @@ int amdtp_motu_set_parameters(struct amdtp_stream *s, unsigned int rate, p->midi_db_count = 0; p->midi_db_interval = rate / MIDI_BYTES_PER_SECOND;
- delay = TRANSFER_DELAY_TICKS; - - // For no-data or empty packets to adjust PCM sampling frequency. - delay += TICKS_PER_SECOND * s->syt_interval / rate; - - p->next_seconds = 0; - p->next_cycles = delay / TICKS_PER_CYCLE; - p->quotient_ticks_per_event = params[s->sfc].quotient_ticks_per_event; - p->remainder_ticks_per_event = params[s->sfc].remainder_ticks_per_event; - p->next_ticks = delay % TICKS_PER_CYCLE; - p->next_accumulated = 0; - return 0; }
@@ -400,47 +364,26 @@ static unsigned int process_ir_ctx_payloads(struct amdtp_stream *s, return pcm_frames; }
-static inline void compute_next_elapse_from_start(struct amdtp_motu *p) +static void write_sph(struct amdtp_motu_cache *cache, __be32 *buffer, unsigned int data_blocks, + unsigned int data_block_quadlets) { - p->next_accumulated += p->remainder_ticks_per_event; - if (p->next_accumulated >= 441) { - p->next_accumulated -= 441; - p->next_ticks++; - } - - p->next_ticks += p->quotient_ticks_per_event; - if (p->next_ticks >= TICKS_PER_CYCLE) { - p->next_ticks -= TICKS_PER_CYCLE; - p->next_cycles++; - } - - if (p->next_cycles >= CYCLES_PER_SECOND) { - p->next_cycles -= CYCLES_PER_SECOND; - p->next_seconds++; - } - - if (p->next_seconds >= IEEE1394_SEC_MODULUS) - p->next_seconds -= IEEE1394_SEC_MODULUS; -} - -static void write_sph(struct amdtp_stream *s, __be32 *buffer, unsigned int data_blocks, - const unsigned int rx_start_cycle) -{ - struct amdtp_motu *p = s->protocol; - unsigned int next_cycles; - unsigned int i; - u32 sph; + unsigned int *event_offsets = cache->event_offsets; + const unsigned int cache_size = cache->size; + unsigned int cache_head = cache->head; + unsigned int base_tick = cache->rx_cycle_count * TICKS_PER_CYCLE; + int i;
for (i = 0; i < data_blocks; i++) { - next_cycles = (rx_start_cycle + p->next_cycles) % CYCLES_PER_SECOND; - sph = ((next_cycles << CIP_SPH_CYCLE_SHIFT) | p->next_ticks) & - (CIP_SPH_CYCLE_MASK | CIP_SPH_OFFSET_MASK); + unsigned int tick = (base_tick + event_offsets[cache_head]) % TICKS_PER_SECOND; + u32 sph = ((tick / TICKS_PER_CYCLE) << CIP_SPH_CYCLE_SHIFT) | (tick % TICKS_PER_CYCLE); *buffer = cpu_to_be32(sph);
- compute_next_elapse_from_start(p); - - buffer += s->data_block_quadlets; + cache_head = (cache_head + 1) % cache_size; + buffer += data_block_quadlets; } + + cache->head = cache_head; + cache->rx_cycle_count = (cache->rx_cycle_count + 1) % CYCLES_PER_SECOND; }
static unsigned int process_it_ctx_payloads(struct amdtp_stream *s, @@ -448,11 +391,13 @@ static unsigned int process_it_ctx_payloads(struct amdtp_stream *s, unsigned int packets, struct snd_pcm_substream *pcm) { - const unsigned int rx_start_cycle = s->domain->processing_cycle.rx_start; struct amdtp_motu *p = s->protocol; unsigned int pcm_frames = 0; int i;
+ if (p->cache->rx_cycle_count == UINT_MAX) + p->cache->rx_cycle_count = (s->domain->processing_cycle.rx_start % CYCLES_PER_SECOND); + // For data block processing. for (i = 0; i < packets; ++i) { const struct pkt_desc *desc = descs + i; @@ -471,7 +416,7 @@ static unsigned int process_it_ctx_payloads(struct amdtp_stream *s,
// TODO: how to interact control messages between userspace?
- write_sph(s, buf, data_blocks, rx_start_cycle); + write_sph(p->cache, buf, data_blocks, s->data_block_quadlets); }
// For tracepoints. diff --git a/sound/firewire/motu/motu-stream.c b/sound/firewire/motu/motu-stream.c index 369002568b2d..43ff5be32b15 100644 --- a/sound/firewire/motu/motu-stream.c +++ b/sound/firewire/motu/motu-stream.c @@ -274,8 +274,13 @@ int snd_motu_stream_start_duplex(struct snd_motu *motu)
motu->cache.tail = 0; motu->cache.tx_cycle_count = UINT_MAX; + motu->cache.head = 0; + motu->cache.rx_cycle_count = UINT_MAX;
- err = amdtp_domain_start(&motu->domain, 0, false, false); + // NOTE: The device requires both of replay; the sequence of the number of data + // blocks per packet, and the sequence of source packet header per data block as + // presentation time. + err = amdtp_domain_start(&motu->domain, 0, true, false); if (err < 0) goto stop_streams;
diff --git a/sound/firewire/motu/motu.h b/sound/firewire/motu/motu.h index 10ba87062e81..674e3dc4e45d 100644 --- a/sound/firewire/motu/motu.h +++ b/sound/firewire/motu/motu.h @@ -44,6 +44,8 @@ struct amdtp_motu_cache { unsigned int size; unsigned int tail; unsigned int tx_cycle_count; + unsigned int head; + unsigned int rx_cycle_count; };
struct snd_motu {
On Wed, 02 Jun 2021 03:34:03 +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 3.
The devices are known to have problems when ALSA firewire-motu driver handles. Many of them generate sound with noise. In the worst case, it generates no sound.
The devices interpret presentation time to decide playback timing. Unlike the syt-aware devices, the devices interpret the presentation time in source packet header (SPH) per data block, instead of the presentation time in syt field of CIP header.
Current implementation of the driver processes the sequence of outgoing packet by computation according to nominal sampling transfer frequency. 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, instead of nominal sampling transfer frequency. For the detail of sequence replay, please refer to a commit 39c2649c71d8 ("ALSA: firewire-lib: replay sequence of incoming packets for outgoing packets"). The sequence replay is done by two levels; the sequence of the number of data blocks per packet, and the sequence of SPH per data blocks in the packet.
Takashi Sakamoto (3): ALSA: firewire-motu: use macro for magic numbers relevant to IEC 61883-1 ALSA: firewire-motu: cache event ticks in source packet header per data block ALSA: firewire-motu: sequence replay for source packet header
Applied all three patches now. Thanks.
Takashi
participants (2)
-
Takashi Iwai -
Takashi Sakamoto