[alsa-devel] [RFC][PATCH 0/6] ALSA: firewire: drop reusing incoming packet parameters for outgoing packet parameters

Takashi Sakamoto

25 Apr 2016 25 Apr '16

2:45 p.m.

Hi,

The purpose of this patchset is to remove a feature of firewire-lib module. I call it as 'full duplex streaming with timestamp synchronization'.

This patchset should accumulated into my previous RFC patchset below: [alsa-devel] [RFC][PATCH 0/3 v2] firewire-lib: support Linux tracing framework to dump a part of packet data http://mailman.alsa-project.org/pipermail/alsa-devel/2016-April/106950.html

The feature is based on a specification of IEC 61883-6. In the specification, an audio function block should reuse some parameters of incoming packets for outgoing packets. Current ALSA firewire stack partly implement it. However, actual audio and music units on IEEE 1394 bus don't expect drivers to perform the specification.

Additionally to the fact, the feature brings inconveniences to handle payloads of isochronous packet in software interrupts.

For these reasons, this patchset removes the feature from ALSA firewire stack. This decision may be a help for future improvement about dynamic interrupt interval of isochronous context, about which Clemens have mentioned[1].

[1][alsa-devel] For firewire drivers, reduce period time down to 2msec? http://mailman.alsa-project.org/pipermail/alsa-devel/2014-April/075696.html

Regards

Takashi Sakamoto (6): ALSA: bebob: drop reusing incoming packet parameter for outgoing packet parameter ALSA: fireworks: drop reusing incoming packet parameter for ougoing packet parameter ALSA: firewire-tascam: drop reusing incoming packet parameter for outgoing packet parameter ALSA: firewire-lib: handle IT/IR contexts in each software context ALSA: firewire-lib: code cleanup for incoming packet handling ALSA: firewire-lib: code cleanup for outgoing packet handling

sound/firewire/amdtp-stream.c | 84 ++++++++---------------- sound/firewire/amdtp-stream.h | 35 ++-------- sound/firewire/bebob/bebob.h | 1 - sound/firewire/bebob/bebob_stream.c | 99 ++++++++--------------------- sound/firewire/fireworks/fireworks.h | 1 - sound/firewire/fireworks/fireworks_stream.c | 84 +++++------------------- sound/firewire/tascam/tascam-stream.c | 26 ++++---- 7 files changed, 87 insertions(+), 243 deletions(-)

-- 2.7.4

Show replies by date

Takashi Sakamoto

25 Apr 25 Apr

2:45 p.m.

New subject: [alsa-devel] [PATCH 1/6] ALSA: bebob: drop reusing incoming packet parameter for outgoing packet parameter

Windows driver for BeBoB-based models mostly wait for transmitted packets, then transfer packets to the models. This looks for the relationship between incoming packets and outgoing packets to synchronize the sequence of presentation timestamp.

However, the sequence between packets of both direction has no relationship. Even if receiving NO-DATA packets, the drivers transfer packets with meaningful value in SYT field. Additionally, the order of starting packets is always the same, independently of the source of clock. The corresponding driver is expected as a generator of presentation timestamp and these models can select it as a source of sampling clock.

This commit drops reusing SYT sequence from ALSA bebob driver. The driver always transfer packets with presentation timestamp generated by ALSA firewire stack, without re-using the sequence of value in SYT field in incoming packets to outgoing packets.

Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/firewire/bebob/bebob.h | 1 - sound/firewire/bebob/bebob_stream.c | 99 ++++++++++--------------------------- 2 files changed, 26 insertions(+), 74 deletions(-)

diff --git a/sound/firewire/bebob/bebob.h b/sound/firewire/bebob/bebob.h index 2a442a7..e7f1bb9 100644 --- a/sound/firewire/bebob/bebob.h +++ b/sound/firewire/bebob/bebob.h @@ -94,7 +94,6 @@ struct snd_bebob {

bool connected;

- struct amdtp_stream *master; struct amdtp_stream tx_stream; struct amdtp_stream rx_stream; struct cmp_connection out_conn; diff --git a/sound/firewire/bebob/bebob_stream.c b/sound/firewire/bebob/bebob_stream.c index 77cbb02..0141813 100644 --- a/sound/firewire/bebob/bebob_stream.c +++ b/sound/firewire/bebob/bebob_stream.c @@ -484,30 +484,6 @@ destroy_both_connections(struct snd_bebob *bebob) }

static int -get_sync_mode(struct snd_bebob *bebob, enum cip_flags *sync_mode) -{ - enum snd_bebob_clock_type src; - int err; - - err = snd_bebob_stream_get_clock_src(bebob, &src); - if (err < 0) - return err; - - switch (src) { - case SND_BEBOB_CLOCK_TYPE_INTERNAL: - case SND_BEBOB_CLOCK_TYPE_EXTERNAL: - *sync_mode = CIP_SYNC_TO_DEVICE; - break; - default: - case SND_BEBOB_CLOCK_TYPE_SYT: - *sync_mode = 0; - break; - } - - return 0; -} - -static int start_stream(struct snd_bebob *bebob, struct amdtp_stream *stream, unsigned int rate) { @@ -584,8 +560,6 @@ end: int snd_bebob_stream_start_duplex(struct snd_bebob *bebob, unsigned int rate) { const struct snd_bebob_rate_spec *rate_spec = bebob->spec->rate; - struct amdtp_stream *master, *slave; - enum cip_flags sync_mode; unsigned int curr_rate; int err = 0;

@@ -593,22 +567,11 @@ int snd_bebob_stream_start_duplex(struct snd_bebob *bebob, unsigned int rate) if (bebob->substreams_counter == 0) goto end;

- err = get_sync_mode(bebob, &sync_mode); - if (err < 0) - goto end; - if (sync_mode == CIP_SYNC_TO_DEVICE) { - master = &bebob->tx_stream; - slave = &bebob->rx_stream; - } else { - master = &bebob->rx_stream; - slave = &bebob->tx_stream; - } - /* * Considering JACK/FFADO streaming: * TODO: This can be removed hwdep functionality becomes popular. */ - err = check_connection_used_by_others(bebob, master); + err = check_connection_used_by_others(bebob, &bebob->rx_stream); if (err < 0) goto end;

@@ -618,11 +581,12 @@ int snd_bebob_stream_start_duplex(struct snd_bebob *bebob, unsigned int rate) * At bus reset, connections should not be broken here. So streams need * to be re-started. This is a reason to use SKIP_INIT_DBC_CHECK flag. */ - if (amdtp_streaming_error(master)) - amdtp_stream_stop(master); - if (amdtp_streaming_error(slave)) - amdtp_stream_stop(slave); - if (!amdtp_stream_running(master) && !amdtp_stream_running(slave)) + if (amdtp_streaming_error(&bebob->rx_stream)) + amdtp_stream_stop(&bebob->rx_stream); + if (amdtp_streaming_error(&bebob->tx_stream)) + amdtp_stream_stop(&bebob->tx_stream); + if (!amdtp_stream_running(&bebob->rx_stream) && + !amdtp_stream_running(&bebob->tx_stream)) break_both_connections(bebob);

/* stop streams if rate is different */ @@ -635,16 +599,13 @@ int snd_bebob_stream_start_duplex(struct snd_bebob *bebob, unsigned int rate) if (rate == 0) rate = curr_rate; if (rate != curr_rate) { - amdtp_stream_stop(master); - amdtp_stream_stop(slave); + amdtp_stream_stop(&bebob->rx_stream); + amdtp_stream_stop(&bebob->tx_stream); break_both_connections(bebob); }

/* master should be always running */ - if (!amdtp_stream_running(master)) { - amdtp_stream_set_sync(sync_mode, master, slave); - bebob->master = master; - + if (!amdtp_stream_running(&bebob->rx_stream)) { /* * NOTE: * If establishing connections at first, Yamaha GO46 @@ -666,7 +627,7 @@ int snd_bebob_stream_start_duplex(struct snd_bebob *bebob, unsigned int rate) if (err < 0) goto end;

- err = start_stream(bebob, master, rate); + err = start_stream(bebob, &bebob->rx_stream, rate); if (err < 0) { dev_err(&bebob->unit->device, "fail to run AMDTP master stream:%d\n", err); @@ -685,15 +646,16 @@ int snd_bebob_stream_start_duplex(struct snd_bebob *bebob, unsigned int rate) dev_err(&bebob->unit->device, "fail to ensure sampling rate: %d\n", err); - amdtp_stream_stop(master); + amdtp_stream_stop(&bebob->rx_stream); break_both_connections(bebob); goto end; } }

/* wait first callback */ - if (!amdtp_stream_wait_callback(master, CALLBACK_TIMEOUT)) { - amdtp_stream_stop(master); + if (!amdtp_stream_wait_callback(&bebob->rx_stream, + CALLBACK_TIMEOUT)) { + amdtp_stream_stop(&bebob->rx_stream); break_both_connections(bebob); err = -ETIMEDOUT; goto end; @@ -701,20 +663,21 @@ int snd_bebob_stream_start_duplex(struct snd_bebob *bebob, unsigned int rate) }

/* start slave if needed */ - if (!amdtp_stream_running(slave)) { - err = start_stream(bebob, slave, rate); + if (!amdtp_stream_running(&bebob->tx_stream)) { + err = start_stream(bebob, &bebob->tx_stream, rate); if (err < 0) { dev_err(&bebob->unit->device, "fail to run AMDTP slave stream:%d\n", err); - amdtp_stream_stop(master); + amdtp_stream_stop(&bebob->rx_stream); break_both_connections(bebob); goto end; }

/* wait first callback */ - if (!amdtp_stream_wait_callback(slave, CALLBACK_TIMEOUT)) { - amdtp_stream_stop(slave); - amdtp_stream_stop(master); + if (!amdtp_stream_wait_callback(&bebob->tx_stream, + CALLBACK_TIMEOUT)) { + amdtp_stream_stop(&bebob->tx_stream); + amdtp_stream_stop(&bebob->rx_stream); break_both_connections(bebob); err = -ETIMEDOUT; } @@ -725,22 +688,12 @@ end:

void snd_bebob_stream_stop_duplex(struct snd_bebob *bebob) { - struct amdtp_stream *master, *slave; - - if (bebob->master == &bebob->rx_stream) { - slave = &bebob->tx_stream; - master = &bebob->rx_stream; - } else { - slave = &bebob->rx_stream; - master = &bebob->tx_stream; - } - if (bebob->substreams_counter == 0) { - amdtp_stream_pcm_abort(master); - amdtp_stream_stop(master); + amdtp_stream_pcm_abort(&bebob->rx_stream); + amdtp_stream_stop(&bebob->rx_stream);

- amdtp_stream_pcm_abort(slave); - amdtp_stream_stop(slave); + amdtp_stream_pcm_abort(&bebob->tx_stream); + amdtp_stream_stop(&bebob->tx_stream);

break_both_connections(bebob); }

-- 2.7.4

Takashi Sakamoto

2:45 p.m.

New subject: [alsa-devel] [PATCH 2/6] ALSA: fireworks: drop reusing incoming packet parameter for ougoing packet parameter

On Fireworks board module of Echo Audio, TSB43Cx43A (IceLynx Micro, iCEM) is used to process payload of isochronous packets. There's an public document of this chip[1]. This document is for firmware programmers to transfer/receive AMDTP with IEC60958 data format, however in clause 2.5, 2.6 and 2.7, we can see system design to utilize the sequence of value in SYT field of CIP header. In clause 2.3, we can see the specification of Audio Master Clock (MCLK) from iCEM.

Well, this clock is actually not used for sampling clock. This can be confirmed when corresponding driver transfer random value as the sequence of SYT field. Even if in this case, the unit generates proper sound.

Additionally, in unique command set for this board module, the format of CIP is changed; for IEC 61883-6 mode which we use, and for Windows Operating System. In the latter mode, the whole 32 bit field in second CIP header from Windows driver is used to represent counter of packets (NO-DATA code is still used for packets without data blocks). If the master clock was physically used by DSP on the board module, the Windows driver must have transferred correct sequence of SYT field.

Furthermore, as long as seeing capacities of AudioFire2, AudioFire4, AudioFire8, AudioFirePre8 and AudioFire12, these models don't support SYT-Match clock source.

Summary, we have no need to relate incoming/outgoing packets. This commit drops reusing SYT sequence of incoming packets for outgoing packets.

[1] Using TSB43Cx43A: S/PDIF over 1394 (2003, Texus Instruments Incorporated) http://www.ti.com/analog/docs/litabsmultiplefilelist.tsp?literatureNumber=sl...

Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/firewire/fireworks/fireworks.h | 1 - sound/firewire/fireworks/fireworks_stream.c | 84 ++++++----------------------- 2 files changed, 17 insertions(+), 68 deletions(-)

diff --git a/sound/firewire/fireworks/fireworks.h b/sound/firewire/fireworks/fireworks.h index 471c772..03ed352 100644 --- a/sound/firewire/fireworks/fireworks.h +++ b/sound/firewire/fireworks/fireworks.h @@ -84,7 +84,6 @@ struct snd_efw { unsigned int pcm_capture_channels[SND_EFW_MULTIPLIER_MODES]; unsigned int pcm_playback_channels[SND_EFW_MULTIPLIER_MODES];

- struct amdtp_stream *master; struct amdtp_stream tx_stream; struct amdtp_stream rx_stream; struct cmp_connection out_conn; diff --git a/sound/firewire/fireworks/fireworks_stream.c b/sound/firewire/fireworks/fireworks_stream.c index 425db8d..ee47924 100644 --- a/sound/firewire/fireworks/fireworks_stream.c +++ b/sound/firewire/fireworks/fireworks_stream.c @@ -121,23 +121,6 @@ destroy_stream(struct snd_efw *efw, struct amdtp_stream *stream) }

static int -get_sync_mode(struct snd_efw *efw, enum cip_flags *sync_mode) -{ - enum snd_efw_clock_source clock_source; - int err; - - err = snd_efw_command_get_clock_source(efw, &clock_source); - if (err < 0) - return err; - - if (clock_source == SND_EFW_CLOCK_SOURCE_SYTMATCH) - return -ENOSYS; - - *sync_mode = CIP_SYNC_TO_DEVICE; - return 0; -} - -static int check_connection_used_by_others(struct snd_efw *efw, struct amdtp_stream *s) { struct cmp_connection *conn; @@ -208,9 +191,6 @@ end:

int snd_efw_stream_start_duplex(struct snd_efw *efw, unsigned int rate) { - struct amdtp_stream *master, *slave; - unsigned int slave_substreams; - enum cip_flags sync_mode; unsigned int curr_rate; int err = 0;

@@ -218,32 +198,19 @@ int snd_efw_stream_start_duplex(struct snd_efw *efw, unsigned int rate) if (efw->playback_substreams == 0 && efw->capture_substreams == 0) goto end;

- err = get_sync_mode(efw, &sync_mode); - if (err < 0) - goto end; - if (sync_mode == CIP_SYNC_TO_DEVICE) { - master = &efw->tx_stream; - slave = &efw->rx_stream; - slave_substreams = efw->playback_substreams; - } else { - master = &efw->rx_stream; - slave = &efw->tx_stream; - slave_substreams = efw->capture_substreams; - } - /* * Considering JACK/FFADO streaming: * TODO: This can be removed hwdep functionality becomes popular. */ - err = check_connection_used_by_others(efw, master); + err = check_connection_used_by_others(efw, &efw->rx_stream); if (err < 0) goto end;

/* packet queueing error */ - if (amdtp_streaming_error(slave)) - stop_stream(efw, slave); - if (amdtp_streaming_error(master)) - stop_stream(efw, master); + if (amdtp_streaming_error(&efw->tx_stream)) + stop_stream(efw, &efw->tx_stream); + if (amdtp_streaming_error(&efw->rx_stream)) + stop_stream(efw, &efw->rx_stream);

/* stop streams if rate is different */ err = snd_efw_command_get_sampling_rate(efw, &curr_rate); @@ -252,20 +219,17 @@ int snd_efw_stream_start_duplex(struct snd_efw *efw, unsigned int rate) if (rate == 0) rate = curr_rate; if (rate != curr_rate) { - stop_stream(efw, slave); - stop_stream(efw, master); + stop_stream(efw, &efw->tx_stream); + stop_stream(efw, &efw->rx_stream); }

/* master should be always running */ - if (!amdtp_stream_running(master)) { - amdtp_stream_set_sync(sync_mode, master, slave); - efw->master = master; - + if (!amdtp_stream_running(&efw->rx_stream)) { err = snd_efw_command_set_sampling_rate(efw, rate); if (err < 0) goto end;

- err = start_stream(efw, master, rate); + err = start_stream(efw, &efw->rx_stream, rate); if (err < 0) { dev_err(&efw->unit->device, "fail to start AMDTP master stream:%d\n", err); @@ -274,12 +238,13 @@ int snd_efw_stream_start_duplex(struct snd_efw *efw, unsigned int rate) }

/* start slave if needed */ - if (slave_substreams > 0 && !amdtp_stream_running(slave)) { - err = start_stream(efw, slave, rate); + if (efw->capture_substreams > 0 && + !amdtp_stream_running(&efw->tx_stream)) { + err = start_stream(efw, &efw->tx_stream, rate); if (err < 0) { dev_err(&efw->unit->device, "fail to start AMDTP slave stream:%d\n", err); - stop_stream(efw, master); + stop_stream(efw, &efw->rx_stream); } } end: @@ -288,26 +253,11 @@ end:

void snd_efw_stream_stop_duplex(struct snd_efw *efw) { - struct amdtp_stream *master, *slave; - unsigned int master_substreams, slave_substreams; - - if (efw->master == &efw->rx_stream) { - slave = &efw->tx_stream; - master = &efw->rx_stream; - slave_substreams = efw->capture_substreams; - master_substreams = efw->playback_substreams; - } else { - slave = &efw->rx_stream; - master = &efw->tx_stream; - slave_substreams = efw->playback_substreams; - master_substreams = efw->capture_substreams; - } - - if (slave_substreams == 0) { - stop_stream(efw, slave); + if (efw->capture_substreams == 0) { + stop_stream(efw, &efw->tx_stream);

- if (master_substreams == 0) - stop_stream(efw, master); + if (efw->playback_substreams == 0) + stop_stream(efw, &efw->rx_stream); } }

-- 2.7.4

Takashi Sakamoto

2:46 p.m.

New subject: [alsa-devel] [PATCH 3/6] ALSA: firewire-tascam: drop reusing incoming packet parameter for outgoing packet parameter

In packet streaming protocol applied to TASCAM FireWire series, the value of SYT field in CIP header is always zero, therefore it has no meaning. There's no need to synchronize packets in both direction for the series.

In current implementation of ALSA firewire stack, driver for the series uses incoming packet parameter for outgoing packet parameter to calculate the number of data blocks. This can be simplified because the task of corresponding driver is to transfer data blocks enough to sampling transfer frequency.

This commit purges support of full duplex synchronization to prevent over-engineering implementation.

Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/firewire/tascam/tascam-stream.c | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/sound/firewire/tascam/tascam-stream.c b/sound/firewire/tascam/tascam-stream.c index 0e6dd5c6..4ad3bd7 100644 --- a/sound/firewire/tascam/tascam-stream.c +++ b/sound/firewire/tascam/tascam-stream.c @@ -381,19 +381,17 @@ int snd_tscm_stream_start_duplex(struct snd_tscm *tscm, unsigned int rate) if (err < 0) return err; if (curr_rate != rate || - amdtp_streaming_error(&tscm->tx_stream) || - amdtp_streaming_error(&tscm->rx_stream)) { + amdtp_streaming_error(&tscm->rx_stream) || + amdtp_streaming_error(&tscm->tx_stream)) { finish_session(tscm);

- amdtp_stream_stop(&tscm->tx_stream); amdtp_stream_stop(&tscm->rx_stream); + amdtp_stream_stop(&tscm->tx_stream);

release_resources(tscm); }

- if (!amdtp_stream_running(&tscm->tx_stream)) { - amdtp_stream_set_sync(CIP_SYNC_TO_DEVICE, - &tscm->tx_stream, &tscm->rx_stream); + if (!amdtp_stream_running(&tscm->rx_stream)) { err = keep_resources(tscm, rate); if (err < 0) goto error; @@ -406,27 +404,27 @@ int snd_tscm_stream_start_duplex(struct snd_tscm *tscm, unsigned int rate) if (err < 0) goto error;

- err = amdtp_stream_start(&tscm->tx_stream, - tscm->tx_resources.channel, + err = amdtp_stream_start(&tscm->rx_stream, + tscm->rx_resources.channel, fw_parent_device(tscm->unit)->max_speed); if (err < 0) goto error;

- if (!amdtp_stream_wait_callback(&tscm->tx_stream, + if (!amdtp_stream_wait_callback(&tscm->rx_stream, CALLBACK_TIMEOUT)) { err = -ETIMEDOUT; goto error; } }

- if (!amdtp_stream_running(&tscm->rx_stream)) { - err = amdtp_stream_start(&tscm->rx_stream, - tscm->rx_resources.channel, + if (!amdtp_stream_running(&tscm->tx_stream)) { + err = amdtp_stream_start(&tscm->tx_stream, + tscm->tx_resources.channel, fw_parent_device(tscm->unit)->max_speed); if (err < 0) goto error;

- if (!amdtp_stream_wait_callback(&tscm->rx_stream, + if (!amdtp_stream_wait_callback(&tscm->tx_stream, CALLBACK_TIMEOUT)) { err = -ETIMEDOUT; goto error; @@ -435,8 +433,8 @@ int snd_tscm_stream_start_duplex(struct snd_tscm *tscm, unsigned int rate)

return 0; error: - amdtp_stream_stop(&tscm->tx_stream); amdtp_stream_stop(&tscm->rx_stream); + amdtp_stream_stop(&tscm->tx_stream);

finish_session(tscm); release_resources(tscm);

-- 2.7.4

Takashi Sakamoto

2:46 p.m.

New subject: [alsa-devel] [PATCH 4/6] ALSA: firewire-lib: handle IT/IR contexts in each software context

In clause 6.3 of IEC 61883-6:2000, there's an explanation about processing of presentation timestamp. In the clause, we can see "If a function block receives a CIP, processes it and subsequently re-transmits it, then the SYT of the outgoing CIP shall be the sum of the incoming SYT and the processing delay." ALSA firewire stack has an implementation to partly satisfy this specification. Developers assumed the stack to perform as an Audio function block[1].

According to the assumption, current implementation of ALSA firewire stack use one software interrupt context to handle both of in/out packets. In most case, this is processed in 1394 OHCI IR context independently of the opposite context. Thus, this implementation uses longer CPU time in the software interrupt context. This is not better for whole system.

Against the assumption, I confirmed that each ASIC for IEC 61883-1/6 doesn't necessarily expect it to the stack. Thus, current implementation of ALSA firewire stack includes over-engineering.

This commit purges the implementation. As a result, packets of one direction are handled in one software interrupt context and spends minimum CPU time.

[1] [alsa-devel] [PATCH 0/8] [RFC] new driver for Echo Audio's Fireworks based devices http://mailman.alsa-project.org/pipermail/alsa-devel/2013-June/062660.html

Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/firewire/amdtp-stream.c | 30 ------------------------------ sound/firewire/amdtp-stream.h | 35 +++++++---------------------------- 2 files changed, 7 insertions(+), 58 deletions(-)

diff --git a/sound/firewire/amdtp-stream.c b/sound/firewire/amdtp-stream.c index a84a9ff..d377dbe 100644 --- a/sound/firewire/amdtp-stream.c +++ b/sound/firewire/amdtp-stream.c @@ -91,7 +91,6 @@ int amdtp_stream_init(struct amdtp_stream *s, struct fw_unit *unit,

init_waitqueue_head(&s->callback_wait); s->callbacked = false; - s->sync_slave = NULL;

s->fmt = fmt; s->process_data_blocks = process_data_blocks; @@ -660,44 +659,17 @@ static void in_stream_callback(struct fw_iso_context *context, u32 tstamp, s->packet_index = -1; break; } - - /* Process sync slave stream */ - if (s->sync_slave && s->sync_slave->callbacked) { - if (handle_out_packet(s->sync_slave, - data_blocks, cycle, syt) < 0) { - s->packet_index = -1; - break; - } - } }

/* Queueing error or detecting discontinuity */ if (s->packet_index < 0) { amdtp_stream_pcm_abort(s); - - /* Abort sync slave. */ - if (s->sync_slave) { - s->sync_slave->packet_index = -1; - amdtp_stream_pcm_abort(s->sync_slave); - } return; }

- /* when sync to device, flush the packets for slave stream */ - if (s->sync_slave && s->sync_slave->callbacked) - fw_iso_context_queue_flush(s->sync_slave->context); - fw_iso_context_queue_flush(s->context); }

-/* processing is done by master callback */ -static void slave_stream_callback(struct fw_iso_context *context, u32 tstamp, - size_t header_length, void *header, - void *private_data) -{ - return; -} - /* this is executed one time */ static void amdtp_stream_first_callback(struct fw_iso_context *context, u32 tstamp, size_t header_length, @@ -714,8 +686,6 @@ static void amdtp_stream_first_callback(struct fw_iso_context *context,

if (s->direction == AMDTP_IN_STREAM) context->callback.sc = in_stream_callback; - else if (s->flags & CIP_SYNC_TO_DEVICE) - context->callback.sc = slave_stream_callback; else context->callback.sc = out_stream_callback;

diff --git a/sound/firewire/amdtp-stream.h b/sound/firewire/amdtp-stream.h index 8775704..da028b0 100644 --- a/sound/firewire/amdtp-stream.h +++ b/sound/firewire/amdtp-stream.h @@ -17,8 +17,6 @@ * @CIP_BLOCKING: In blocking mode, each packet contains either zero or * SYT_INTERVAL samples, with these two types alternating so that * the overall sample rate comes out right. - * @CIP_SYNC_TO_DEVICE: In sync to device mode, time stamp in out packets is - * generated by in packets. Defaultly this driver generates timestamp. * @CIP_EMPTY_WITH_TAG0: Only for in-stream. Empty in-packets have TAG0. * @CIP_DBC_IS_END_EVENT: Only for in-stream. The value of dbc in an in-packet * corresponds to the end of event in the packet. Out of IEC 61883. @@ -37,14 +35,13 @@ enum cip_flags { CIP_NONBLOCKING = 0x00, CIP_BLOCKING = 0x01, - CIP_SYNC_TO_DEVICE = 0x02, - CIP_EMPTY_WITH_TAG0 = 0x04, - CIP_DBC_IS_END_EVENT = 0x08, - CIP_WRONG_DBS = 0x10, - CIP_SKIP_DBC_ZERO_CHECK = 0x20, - CIP_SKIP_INIT_DBC_CHECK = 0x40, - CIP_EMPTY_HAS_WRONG_DBC = 0x80, - CIP_JUMBO_PAYLOAD = 0x100, + CIP_EMPTY_WITH_TAG0 = 0x02, + CIP_DBC_IS_END_EVENT = 0x04, + CIP_WRONG_DBS = 0x08, + CIP_SKIP_DBC_ZERO_CHECK = 0x10, + CIP_SKIP_INIT_DBC_CHECK = 0x20, + CIP_EMPTY_HAS_WRONG_DBC = 0x40, + CIP_JUMBO_PAYLOAD = 0x80, };

/** @@ -137,7 +134,6 @@ struct amdtp_stream { /* To wait for first packet. */ bool callbacked; wait_queue_head_t callback_wait; - struct amdtp_stream *sync_slave;

/* For backends to process data blocks. */ void *protocol; @@ -223,23 +219,6 @@ static inline bool cip_sfc_is_base_44100(enum cip_sfc sfc) return sfc & 1; }

-static inline void amdtp_stream_set_sync(enum cip_flags sync_mode, - struct amdtp_stream *master, - struct amdtp_stream *slave) -{ - if (sync_mode == CIP_SYNC_TO_DEVICE) { - master->flags |= CIP_SYNC_TO_DEVICE; - slave->flags |= CIP_SYNC_TO_DEVICE; - master->sync_slave = slave; - } else { - master->flags &= ~CIP_SYNC_TO_DEVICE; - slave->flags &= ~CIP_SYNC_TO_DEVICE; - master->sync_slave = NULL; - } - - slave->sync_slave = NULL; -} - /** * amdtp_stream_wait_callback - sleep till callbacked or timeout * @s: the AMDTP stream

-- 2.7.4

Takashi Sakamoto

2:46 p.m.

New subject: [alsa-devel] [PATCH 5/6] ALSA: firewire-lib: code cleanup for incoming packet handling

In previous commit, this module has no need to re-use parameters in incoming packets for outgoing packets anymore. This commit arranges some needless codes for incoming packet processing.

Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/firewire/amdtp-stream.c | 38 ++++++++++++++++++-------------------- 1 file changed, 18 insertions(+), 20 deletions(-)

diff --git a/sound/firewire/amdtp-stream.c b/sound/firewire/amdtp-stream.c index d377dbe..a49459d 100644 --- a/sound/firewire/amdtp-stream.c +++ b/sound/firewire/amdtp-stream.c @@ -447,17 +447,18 @@ static int handle_out_packet(struct amdtp_stream *s, unsigned int data_blocks, }

static int handle_in_packet(struct amdtp_stream *s, - unsigned int payload_quadlets, __be32 *buffer, - unsigned int *data_blocks, unsigned int cycle, - unsigned int syt) + unsigned int payload_quadlets, unsigned int cycle) { + __be32 *buffer; u32 cip_header[2]; - unsigned int fmt, fdf; + unsigned int fmt, fdf, syt; unsigned int data_block_quadlets, data_block_counter, dbc_interval; + unsigned int data_blocks; struct snd_pcm_substream *pcm; unsigned int pcm_frames; bool lost;

+ buffer = s->buffer.packets[s->packet_index].buffer; cip_header[0] = be32_to_cpu(buffer[0]); cip_header[1] = be32_to_cpu(buffer[1]);

@@ -472,7 +473,7 @@ static int handle_in_packet(struct amdtp_stream *s, dev_info_ratelimited(&s->unit->device, "Invalid CIP header for AMDTP: %08X:%08X\n", cip_header[0], cip_header[1]); - *data_blocks = 0; + data_blocks = 0; pcm_frames = 0; goto end; } @@ -483,7 +484,7 @@ static int handle_in_packet(struct amdtp_stream *s, dev_info_ratelimited(&s->unit->device, "Detect unexpected protocol: %08x %08x\n", cip_header[0], cip_header[1]); - *data_blocks = 0; + data_blocks = 0; pcm_frames = 0; goto end; } @@ -492,7 +493,7 @@ static int handle_in_packet(struct amdtp_stream *s, fdf = (cip_header[1] & CIP_FDF_MASK) >> CIP_FDF_SHIFT; if (payload_quadlets < 3 || (fmt == CIP_FMT_AM && fdf == AMDTP_FDF_NO_DATA)) { - *data_blocks = 0; + data_blocks = 0; } else { data_block_quadlets = (cip_header[0] & CIP_DBS_MASK) >> CIP_DBS_SHIFT; @@ -506,12 +507,12 @@ static int handle_in_packet(struct amdtp_stream *s, if (s->flags & CIP_WRONG_DBS) data_block_quadlets = s->data_block_quadlets;

- *data_blocks = (payload_quadlets - 2) / data_block_quadlets; + data_blocks = (payload_quadlets - 2) / data_block_quadlets; }

/* Check data block counter continuity */ data_block_counter = cip_header[0] & CIP_DBC_MASK; - if (*data_blocks == 0 && (s->flags & CIP_EMPTY_HAS_WRONG_DBC) && + if (data_blocks == 0 && (s->flags & CIP_EMPTY_HAS_WRONG_DBC) && s->data_block_counter != UINT_MAX) data_block_counter = s->data_block_counter;

@@ -522,10 +523,10 @@ static int handle_in_packet(struct amdtp_stream *s, } else if (!(s->flags & CIP_DBC_IS_END_EVENT)) { lost = data_block_counter != s->data_block_counter; } else { - if ((*data_blocks > 0) && (s->tx_dbc_interval > 0)) + if (data_blocks > 0 && s->tx_dbc_interval > 0) dbc_interval = s->tx_dbc_interval; else - dbc_interval = *data_blocks; + dbc_interval = data_blocks;

lost = data_block_counter != ((s->data_block_counter + dbc_interval) & 0xff); @@ -538,13 +539,14 @@ static int handle_in_packet(struct amdtp_stream *s, return -EIO; }

- pcm_frames = s->process_data_blocks(s, buffer + 2, *data_blocks, &syt); + syt = be32_to_cpu(buffer[1]) & CIP_SYT_MASK; + pcm_frames = s->process_data_blocks(s, buffer + 2, data_blocks, &syt);

if (s->flags & CIP_DBC_IS_END_EVENT) s->data_block_counter = data_block_counter; else s->data_block_counter = - (data_block_counter + *data_blocks) & 0xff; + (data_block_counter + data_blocks) & 0xff; end: if (queue_in_packet(s) < 0) return -EIO; @@ -618,10 +620,9 @@ static void in_stream_callback(struct fw_iso_context *context, u32 tstamp, void *private_data) { struct amdtp_stream *s = private_data; - unsigned int p, syt, packets; + unsigned int p, packets; unsigned int payload_quadlets, max_payload_quadlets; - unsigned int data_blocks; - __be32 *buffer, *headers = header; + __be32 *headers = header; u32 cycle;

if (s->packet_index < 0) @@ -640,7 +641,6 @@ static void in_stream_callback(struct fw_iso_context *context, u32 tstamp,

for (p = 0; p < packets; p++) { cycle = increment_cycle_count(cycle, 1); - buffer = s->buffer.packets[s->packet_index].buffer;

/* The number of quadlets in this packet */ payload_quadlets = @@ -653,9 +653,7 @@ static void in_stream_callback(struct fw_iso_context *context, u32 tstamp, break; }

- syt = be32_to_cpu(buffer[1]) & CIP_SYT_MASK; - if (handle_in_packet(s, payload_quadlets, buffer, - &data_blocks, cycle, syt) < 0) { + if (handle_in_packet(s, payload_quadlets, cycle) < 0) { s->packet_index = -1; break; }

-- 2.7.4

Takashi Sakamoto

2:46 p.m.

New subject: [alsa-devel] [PATCH 6/6] ALSA: firewire-lib: code cleanup for outgoing packet handling

In previous commit, this module has no need to re-use parameters in incoming packets for outgoing packets anymore. This commit arranges some needless codes for outgoing packet processing.

Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/firewire/amdtp-stream.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/sound/firewire/amdtp-stream.c b/sound/firewire/amdtp-stream.c index a49459d..12a8354 100644 --- a/sound/firewire/amdtp-stream.c +++ b/sound/firewire/amdtp-stream.c @@ -411,15 +411,18 @@ static inline int queue_in_packet(struct amdtp_stream *s) amdtp_stream_get_max_payload(s), false); }

-static int handle_out_packet(struct amdtp_stream *s, unsigned int data_blocks, - unsigned int cycle, unsigned int syt) +static int handle_out_packet(struct amdtp_stream *s, unsigned int cycle) { __be32 *buffer; + unsigned int syt; + unsigned int data_blocks; unsigned int payload_length; unsigned int pcm_frames; struct snd_pcm_substream *pcm;

buffer = s->buffer.packets[s->packet_index].buffer; + syt = calculate_syt(s, cycle); + data_blocks = calculate_data_blocks(s, syt); pcm_frames = s->process_data_blocks(s, buffer + 2, data_blocks, &syt);

buffer[0] = cpu_to_be32(ACCESS_ONCE(s->source_node_id_field) | @@ -442,7 +445,6 @@ static int handle_out_packet(struct amdtp_stream *s, unsigned int data_blocks, if (pcm && pcm_frames > 0) update_pcm_pointers(s, pcm, pcm_frames);

- /* No need to return the number of handled data blocks. */ return 0; }

@@ -588,8 +590,7 @@ static void out_stream_callback(struct fw_iso_context *context, u32 tstamp, void *private_data) { struct amdtp_stream *s = private_data; - unsigned int i, syt, packets = header_length / 4; - unsigned int data_blocks; + unsigned int i, packets = header_length / 4; u32 cycle;

if (s->packet_index < 0) @@ -602,10 +603,7 @@ static void out_stream_callback(struct fw_iso_context *context, u32 tstamp,

for (i = 0; i < packets; ++i) { cycle = increment_cycle_count(cycle, 1); - syt = calculate_syt(s, cycle); - data_blocks = calculate_data_blocks(s, syt); - - if (handle_out_packet(s, data_blocks, cycle, syt) < 0) { + if (handle_out_packet(s, cycle) < 0) { s->packet_index = -1; amdtp_stream_pcm_abort(s); return;

-- 2.7.4

3163

Age (days ago)

3163

Last active (days ago)

List overview

Download

6 comments

1 participants

tags (0)

participants (1)

Takashi Sakamoto