[alsa-devel] [PATCH 0/7] ALSA: drop reuse of incoming packet parameter for outgoing packet parameter

newer
[alsa-devel] [PATCH 0/5] ASoC:...

older
[alsa-devel] [PATCH] ALSA: pcm:...

Takashi Sakamoto

9 May 2016 9 May '16

4:15 p.m.

Hi,

This patchset updates my former RFC, and go for merging to Linux 4.7.

[alsa-devel] [RFC][PATCH 0/6] ALSA: firewire: drop reusing incoming packet parameters for outgoing packet parameters http://mailman.alsa-project.org/pipermail/alsa-devel/2016-April/107290.html

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

The feature is based on our understanding of IEC 61883-6. In the specification, a 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 corresponding drivers to satisfy the feature.

On the other hand, the feature brings inconveniences to this module. When this feature is enabled, a software IRQ context processes packets for both of IR/IT contexts. This consumes more time over local CPUs than handling one context. Furthermore, cycle count for outgoing packets becomes improper because they're for incoming packets.

For these reasons, this patchset removes the feature from ALSA firewire stack.

Changes: - The last patch is newly added.

Regards

Takashi Sakamoto (7): ALSA: bebob: drop reuse of incoming packet parameter for outgoing packet parameter ALSA: fireworks: drop reuse of incoming packet parameter for ougoing packet parameter ALSA: firewire-tascam: drop reuse of incoming packet parameter for outgoing packet parameter ALSA: firewire-lib: handle IT/IR contexts in each software interrupt context ALSA: firewire-lib: code cleanup for incoming packet handling ALSA: firewire-lib: code cleanup for outgoing packet handling ALSA: firewire-lib: enable the same feature as CIP_SKIP_INIT_DBC_CHECK flag

sound/firewire/amdtp-stream.c | 98 +++++++++------------------ sound/firewire/amdtp-stream.h | 36 ++-------- sound/firewire/bebob/bebob.h | 1 - sound/firewire/bebob/bebob_stream.c | 101 +++++++--------------------- sound/firewire/digi00x/amdtp-dot.c | 2 +- sound/firewire/fireworks/fireworks.h | 1 - sound/firewire/fireworks/fireworks_stream.c | 84 +++++------------------ sound/firewire/oxfw/oxfw-stream.c | 3 +- sound/firewire/tascam/tascam-stream.c | 26 ++++--- 9 files changed, 94 insertions(+), 258 deletions(-)

-- 2.7.4

Show replies by date

Takashi Sakamoto

9 May 9 May

4:15 p.m.

New subject: [alsa-devel] [PATCH 1/7] ALSA: bebob: drop reuse of 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

4:15 p.m.

New subject: [alsa-devel] [PATCH 2/7] ALSA: fireworks: drop reuse of 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

4:15 p.m.

New subject: [alsa-devel] [PATCH 3/7] ALSA: firewire-tascam: drop reuse of 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

4:15 p.m.

New subject: [alsa-devel] [PATCH 4/7] ALSA: firewire-lib: handle IT/IR contexts in each software interrupt 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].

Following 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 | 40 ++++------------------------------------ sound/firewire/amdtp-stream.h | 35 +++++++---------------------------- 2 files changed, 11 insertions(+), 64 deletions(-)

diff --git a/sound/firewire/amdtp-stream.c b/sound/firewire/amdtp-stream.c index a22e559..3468419 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; @@ -650,54 +649,25 @@ static void in_stream_callback(struct fw_iso_context *context, u32 tstamp, dev_err(&s->unit->device, "Detect jumbo payload: %02x %02x\n", payload_quadlets, max_payload_quadlets); - s->packet_index = -1; break; }

syt = be32_to_cpu(buffer[1]) & CIP_SYT_MASK; if (handle_in_packet(s, payload_quadlets, buffer, - &data_blocks, cycle, syt) < 0) { - s->packet_index = -1; + &data_blocks, cycle, syt) < 0) 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) { + /* Queueing error or detecting invalid payload. */ + if (p < packets) { + s->packet_index = -1; 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 +684,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

4:15 p.m.

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

In previous commit, this module has no need to reuse parameters of 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 | 44 +++++++++++++++++++++---------------------- 1 file changed, 21 insertions(+), 23 deletions(-)

diff --git a/sound/firewire/amdtp-stream.c b/sound/firewire/amdtp-stream.c index 3468419..f1ebb7b 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 i, 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) @@ -638,13 +639,12 @@ static void in_stream_callback(struct fw_iso_context *context, u32 tstamp, /* For buffer-over-run prevention. */ max_payload_quadlets = amdtp_stream_get_max_payload(s) / 4;

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

/* The number of quadlets in this packet */ payload_quadlets = - (be32_to_cpu(headers[p]) >> ISO_DATA_LENGTH_SHIFT) / 4; + (be32_to_cpu(headers[i]) >> ISO_DATA_LENGTH_SHIFT) / 4; if (payload_quadlets > max_payload_quadlets) { dev_err(&s->unit->device, "Detect jumbo payload: %02x %02x\n", @@ -652,14 +652,12 @@ 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) break; }

/* Queueing error or detecting invalid payload. */ - if (p < packets) { + if (i < packets) { s->packet_index = -1; amdtp_stream_pcm_abort(s); return;

-- 2.7.4

Takashi Sakamoto

4:15 p.m.

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

In previous commit, this module has no need to reuse parameters of 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 | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/sound/firewire/amdtp-stream.c b/sound/firewire/amdtp-stream.c index f1ebb7b..6db2a73 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) | @@ -588,8 +591,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 +604,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

Takashi Sakamoto

4:15 p.m.

New subject: [alsa-devel] [PATCH 7/7] ALSA: firewire-lib: enable the same feature as CIP_SKIP_INIT_DBC_CHECK flag

In former commit, drivers in ALSA firewire stack always starts IT context before IR context. If IR context starts after packets are transmitted by peer unit, packet discontinuity may be detected because the context starts in the middle of packet streaming. This situation is rare because IT context usually starts immediately. However, it's better to solve this issue. This is suppressed with CIP_SKIP_INIT_DBC_CHECK flag.

This commit enables the same feature as CIP_SKIP_INIT_DBC_CHECK.

Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/firewire/amdtp-stream.c | 3 +-- sound/firewire/amdtp-stream.h | 7 ++----- sound/firewire/bebob/bebob_stream.c | 2 -- sound/firewire/digi00x/amdtp-dot.c | 2 +- sound/firewire/oxfw/oxfw-stream.c | 3 +-- 5 files changed, 5 insertions(+), 12 deletions(-)

diff --git a/sound/firewire/amdtp-stream.c b/sound/firewire/amdtp-stream.c index 6db2a73..830a95c 100644 --- a/sound/firewire/amdtp-stream.c +++ b/sound/firewire/amdtp-stream.c @@ -723,8 +723,7 @@ int amdtp_stream_start(struct amdtp_stream *s, int channel, int speed) goto err_unlock; }

- if (s->direction == AMDTP_IN_STREAM && - s->flags & CIP_SKIP_INIT_DBC_CHECK) + if (s->direction == AMDTP_IN_STREAM) s->data_block_counter = UINT_MAX; else s->data_block_counter = 0; diff --git a/sound/firewire/amdtp-stream.h b/sound/firewire/amdtp-stream.h index da028b0..349c405 100644 --- a/sound/firewire/amdtp-stream.h +++ b/sound/firewire/amdtp-stream.h @@ -24,8 +24,6 @@ * The value of data_block_quadlets is used instead of reported value. * @CIP_SKIP_DBC_ZERO_CHECK: Only for in-stream. Packets with zero in dbc is * skipped for detecting discontinuity. - * @CIP_SKIP_INIT_DBC_CHECK: Only for in-stream. The value of dbc in first - * packet is not continuous from an initial value. * @CIP_EMPTY_HAS_WRONG_DBC: Only for in-stream. The value of dbc in empty * packet is wrong but the others are correct. * @CIP_JUMBO_PAYLOAD: Only for in-stream. The number of data blocks in an @@ -39,9 +37,8 @@ enum cip_flags { 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, + CIP_EMPTY_HAS_WRONG_DBC = 0x20, + CIP_JUMBO_PAYLOAD = 0x40, };

/** diff --git a/sound/firewire/bebob/bebob_stream.c b/sound/firewire/bebob/bebob_stream.c index 0141813..4d3034a 100644 --- a/sound/firewire/bebob/bebob_stream.c +++ b/sound/firewire/bebob/bebob_stream.c @@ -526,8 +526,6 @@ int snd_bebob_stream_init_duplex(struct snd_bebob *bebob) goto end; }

- bebob->tx_stream.flags |= CIP_SKIP_INIT_DBC_CHECK; - /* * BeBoB v3 transfers packets with these qurks: * - In the beginning of streaming, the value of dbc is incremented diff --git a/sound/firewire/digi00x/amdtp-dot.c b/sound/firewire/digi00x/amdtp-dot.c index 0ac92ab..b3cffd0 100644 --- a/sound/firewire/digi00x/amdtp-dot.c +++ b/sound/firewire/digi00x/amdtp-dot.c @@ -421,7 +421,7 @@ int amdtp_dot_init(struct amdtp_stream *s, struct fw_unit *unit,

/* Use different mode between incoming/outgoing. */ if (dir == AMDTP_IN_STREAM) { - flags = CIP_NONBLOCKING | CIP_SKIP_INIT_DBC_CHECK; + flags = CIP_NONBLOCKING; process_data_blocks = process_tx_data_blocks; } else { flags = CIP_BLOCKING; diff --git a/sound/firewire/oxfw/oxfw-stream.c b/sound/firewire/oxfw/oxfw-stream.c index 7cb5743..d9361f3 100644 --- a/sound/firewire/oxfw/oxfw-stream.c +++ b/sound/firewire/oxfw/oxfw-stream.c @@ -242,8 +242,7 @@ int snd_oxfw_stream_init_simplex(struct snd_oxfw *oxfw, * blocks than IEC 61883-6 defines. */ if (stream == &oxfw->tx_stream) { - oxfw->tx_stream.flags |= CIP_SKIP_INIT_DBC_CHECK | - CIP_JUMBO_PAYLOAD; + oxfw->tx_stream.flags |= CIP_JUMBO_PAYLOAD; if (oxfw->wrong_dbs) oxfw->tx_stream.flags |= CIP_WRONG_DBS; }

-- 2.7.4

Takashi Iwai

10 May 10 May

5:04 p.m.

New subject: [alsa-devel] [PATCH 0/7] ALSA: drop reuse of incoming packet parameter for outgoing packet parameter

On Mon, 09 May 2016 16:15:49 +0200, Takashi Sakamoto wrote:

...

Hi,

This patchset updates my former RFC, and go for merging to Linux 4.7.

[alsa-devel] [RFC][PATCH 0/6] ALSA: firewire: drop reusing incoming packet parameters for outgoing packet parameters http://mailman.alsa-project.org/pipermail/alsa-devel/2016-April/107290.html

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

The feature is based on our understanding of IEC 61883-6. In the specification, a 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 corresponding drivers to satisfy the feature.

On the other hand, the feature brings inconveniences to this module. When this feature is enabled, a software IRQ context processes packets for both of IR/IT contexts. This consumes more time over local CPUs than handling one context. Furthermore, cycle count for outgoing packets becomes improper because they're for incoming packets.

For these reasons, this patchset removes the feature from ALSA firewire stack.

Changes:

The last patch is newly added.

Regards

Takashi Sakamoto (7): ALSA: bebob: drop reuse of incoming packet parameter for outgoing packet parameter ALSA: fireworks: drop reuse of incoming packet parameter for ougoing packet parameter ALSA: firewire-tascam: drop reuse of incoming packet parameter for outgoing packet parameter ALSA: firewire-lib: handle IT/IR contexts in each software interrupt context ALSA: firewire-lib: code cleanup for incoming packet handling ALSA: firewire-lib: code cleanup for outgoing packet handling ALSA: firewire-lib: enable the same feature as CIP_SKIP_INIT_DBC_CHECK flag

Applied all patches now. Thanks.

Takashi

3148

Age (days ago)

3149

Last active (days ago)

List overview

Download

8 comments

2 participants

tags (0)

participants (2)

Takashi Iwai
Takashi Sakamoto