[alsa-devel] [PATCH 0/6] ALSA: firewire: handle several IR/IT context in callback of the IT context
Hi,
This patchset is a part of my continuous work to improve ALSA IEC 61883-1/6 packet streaming engine for clock-recovery, addressed in my previous message: https://mailman.alsa-project.org/pipermail/alsa-devel/2019-August/153388.htm...
For the clock-recovery, information in the sequence of tx packet from device should be used to generate the sequence of rx packet to the device. In current implementation of the engine, the packets are processed in different tasklet contexts for each IR/IT context. This is inconvenient to bypass information between these IR/IT contexts.
In this patchset, the engine is improved to process all of IR/IT contexts in the same domain by a tasklet context for one of IT context. For convenience, the IT context is called as 'IRQ target'. As a result, 1394 OHCI controller is managed to generate hardware IRQ just for the IRQ target. All of rx/tx packets are processed in tasklet for the hardware IRQ.
Takashi Sakamoto (6): ALSA: firewire-lib: add irq_target member into amdtp_domain struct ALSA: firewire-lib: replace pointer callback to flush isoc contexts in AMDTP domain ALSA: firewire-lib: replace ack callback to flush isoc contexts in AMDTP domain ALSA: firewire-lib: cancel flushing isoc context in the laste step to process context callback ALSA: firewire-lib: handle several AMDTP streams in callback handler of IRQ target ALSA: firewire-lib: postpone to start IR context
sound/firewire/amdtp-stream.c | 329 +++++++++++++++----- sound/firewire/amdtp-stream.h | 17 +- sound/firewire/bebob/bebob_pcm.c | 18 +- sound/firewire/bebob/bebob_stream.c | 10 +- sound/firewire/dice/dice-pcm.c | 8 +- sound/firewire/dice/dice-stream.c | 2 +- sound/firewire/digi00x/digi00x-pcm.c | 8 +- sound/firewire/digi00x/digi00x-stream.c | 2 +- sound/firewire/fireface/ff-pcm.c | 8 +- sound/firewire/fireface/ff-stream.c | 10 +- sound/firewire/fireworks/fireworks_pcm.c | 10 +- sound/firewire/fireworks/fireworks_stream.c | 2 +- sound/firewire/motu/motu-pcm.c | 8 +- sound/firewire/motu/motu-stream.c | 2 +- sound/firewire/oxfw/oxfw-pcm.c | 8 +- sound/firewire/oxfw/oxfw-stream.c | 2 +- sound/firewire/tascam/tascam-pcm.c | 8 +- sound/firewire/tascam/tascam-stream.c | 2 +- 18 files changed, 325 insertions(+), 129 deletions(-)
This commit is a preparation to handle several IR/IT contexts in the same domain by tasklet context for one of the IT context. Such IT context is stored to AMDTP domain structure as 'irq_target'.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/firewire/amdtp-stream.h | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/sound/firewire/amdtp-stream.h b/sound/firewire/amdtp-stream.h index 344818e936df..f92397a2f35f 100644 --- a/sound/firewire/amdtp-stream.h +++ b/sound/firewire/amdtp-stream.h @@ -279,6 +279,8 @@ struct amdtp_domain {
unsigned int events_per_period; unsigned int events_per_buffer; + + struct amdtp_stream *irq_target; };
int amdtp_domain_init(struct amdtp_domain *d);
An isoc context for AMDTP stream is flushed to queue packet by a call of pcm.pointer. This commit extends this for AMDTP domain.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/firewire/amdtp-stream.c | 51 ++++++++++++++---------- sound/firewire/amdtp-stream.h | 3 +- sound/firewire/bebob/bebob_pcm.c | 14 ++++--- sound/firewire/dice/dice-pcm.c | 4 +- sound/firewire/digi00x/digi00x-pcm.c | 4 +- sound/firewire/fireface/ff-pcm.c | 4 +- sound/firewire/fireworks/fireworks_pcm.c | 6 ++- sound/firewire/motu/motu-pcm.c | 4 +- sound/firewire/oxfw/oxfw-pcm.c | 4 +- sound/firewire/tascam/tascam-pcm.c | 4 +- 10 files changed, 56 insertions(+), 42 deletions(-)
diff --git a/sound/firewire/amdtp-stream.c b/sound/firewire/amdtp-stream.c index 7486eec4d958..23677b805b05 100644 --- a/sound/firewire/amdtp-stream.c +++ b/sound/firewire/amdtp-stream.c @@ -1099,35 +1099,44 @@ static int amdtp_stream_start(struct amdtp_stream *s, int channel, int speed, }
/** - * amdtp_stream_pcm_pointer - get the PCM buffer position + * amdtp_domain_stream_pcm_pointer - get the PCM buffer position + * @d: the AMDTP domain. * @s: the AMDTP stream that transports the PCM data * * Returns the current buffer position, in frames. */ -unsigned long amdtp_stream_pcm_pointer(struct amdtp_stream *s) +unsigned long amdtp_domain_stream_pcm_pointer(struct amdtp_domain *d, + struct amdtp_stream *s) { - /* - * This function is called in software IRQ context of period_tasklet or - * process context. - * - * When the software IRQ context was scheduled by software IRQ context - * of IR/IT contexts, queued packets were already handled. Therefore, - * no need to flush the queue in buffer anymore. - * - * When the process context reach here, some packets will be already - * queued in the buffer. These packets should be handled immediately - * to keep better granularity of PCM pointer. - * - * Later, the process context will sometimes schedules software IRQ - * context of the period_tasklet. Then, no need to flush the queue by - * the same reason as described for IR/IT contexts. - */ - if (!in_interrupt() && amdtp_stream_running(s)) - fw_iso_context_flush_completions(s->context); + struct amdtp_stream *irq_target = d->irq_target; + + if (irq_target && amdtp_stream_running(irq_target)) { + // This function is called in software IRQ context of + // period_tasklet or process context. + // + // When the software IRQ context was scheduled by software IRQ + // context of IT contexts, queued packets were already handled. + // Therefore, no need to flush the queue in buffer furthermore. + // + // When the process context reach here, some packets will be + // already queued in the buffer. These packets should be handled + // immediately to keep better granularity of PCM pointer. + // + // Later, the process context will sometimes schedules software + // IRQ context of the period_tasklet. Then, no need to flush the + // queue by the same reason as described in the above + if (!in_interrupt()) { + // Queued packet should be processed without any kernel + // preemption to keep latency against bus cycle. + preempt_disable(); + fw_iso_context_flush_completions(irq_target->context); + preempt_enable(); + } + }
return READ_ONCE(s->pcm_buffer_pointer); } -EXPORT_SYMBOL(amdtp_stream_pcm_pointer); +EXPORT_SYMBOL_GPL(amdtp_domain_stream_pcm_pointer);
/** * amdtp_stream_pcm_ack - acknowledge queued PCM frames diff --git a/sound/firewire/amdtp-stream.h b/sound/firewire/amdtp-stream.h index f92397a2f35f..ba0bbeddfdcb 100644 --- a/sound/firewire/amdtp-stream.h +++ b/sound/firewire/amdtp-stream.h @@ -198,7 +198,6 @@ int amdtp_stream_add_pcm_hw_constraints(struct amdtp_stream *s, struct snd_pcm_runtime *runtime);
void amdtp_stream_pcm_prepare(struct amdtp_stream *s); -unsigned long amdtp_stream_pcm_pointer(struct amdtp_stream *s); int amdtp_stream_pcm_ack(struct amdtp_stream *s); void amdtp_stream_pcm_abort(struct amdtp_stream *s);
@@ -302,4 +301,6 @@ static inline int amdtp_domain_set_events_per_period(struct amdtp_domain *d, return 0; }
+unsigned long amdtp_domain_stream_pcm_pointer(struct amdtp_domain *d, + struct amdtp_stream *s); #endif diff --git a/sound/firewire/bebob/bebob_pcm.c b/sound/firewire/bebob/bebob_pcm.c index 8b2e0ceffe82..dc15ea8d0dc5 100644 --- a/sound/firewire/bebob/bebob_pcm.c +++ b/sound/firewire/bebob/bebob_pcm.c @@ -313,17 +313,19 @@ pcm_playback_trigger(struct snd_pcm_substream *substream, int cmd) return 0; }
-static snd_pcm_uframes_t -pcm_capture_pointer(struct snd_pcm_substream *sbstrm) +static snd_pcm_uframes_t pcm_capture_pointer(struct snd_pcm_substream *sbstrm) { struct snd_bebob *bebob = sbstrm->private_data; - return amdtp_stream_pcm_pointer(&bebob->tx_stream); + + return amdtp_domain_stream_pcm_pointer(&bebob->domain, + &bebob->tx_stream); } -static snd_pcm_uframes_t -pcm_playback_pointer(struct snd_pcm_substream *sbstrm) +static snd_pcm_uframes_t pcm_playback_pointer(struct snd_pcm_substream *sbstrm) { struct snd_bebob *bebob = sbstrm->private_data; - return amdtp_stream_pcm_pointer(&bebob->rx_stream); + + return amdtp_domain_stream_pcm_pointer(&bebob->domain, + &bebob->rx_stream); }
static int pcm_capture_ack(struct snd_pcm_substream *substream) diff --git a/sound/firewire/dice/dice-pcm.c b/sound/firewire/dice/dice-pcm.c index 7c0c34c5bd47..81b78e7d181d 100644 --- a/sound/firewire/dice/dice-pcm.c +++ b/sound/firewire/dice/dice-pcm.c @@ -379,14 +379,14 @@ static snd_pcm_uframes_t capture_pointer(struct snd_pcm_substream *substream) struct snd_dice *dice = substream->private_data; struct amdtp_stream *stream = &dice->tx_stream[substream->pcm->device];
- return amdtp_stream_pcm_pointer(stream); + return amdtp_domain_stream_pcm_pointer(&dice->domain, stream); } static snd_pcm_uframes_t playback_pointer(struct snd_pcm_substream *substream) { struct snd_dice *dice = substream->private_data; struct amdtp_stream *stream = &dice->rx_stream[substream->pcm->device];
- return amdtp_stream_pcm_pointer(stream); + return amdtp_domain_stream_pcm_pointer(&dice->domain, stream); }
static int capture_ack(struct snd_pcm_substream *substream) diff --git a/sound/firewire/digi00x/digi00x-pcm.c b/sound/firewire/digi00x/digi00x-pcm.c index c9a833dff20d..f6a2053d5f10 100644 --- a/sound/firewire/digi00x/digi00x-pcm.c +++ b/sound/firewire/digi00x/digi00x-pcm.c @@ -301,14 +301,14 @@ static snd_pcm_uframes_t pcm_capture_pointer(struct snd_pcm_substream *sbstrm) { struct snd_dg00x *dg00x = sbstrm->private_data;
- return amdtp_stream_pcm_pointer(&dg00x->tx_stream); + return amdtp_domain_stream_pcm_pointer(&dg00x->domain, &dg00x->tx_stream); }
static snd_pcm_uframes_t pcm_playback_pointer(struct snd_pcm_substream *sbstrm) { struct snd_dg00x *dg00x = sbstrm->private_data;
- return amdtp_stream_pcm_pointer(&dg00x->rx_stream); + return amdtp_domain_stream_pcm_pointer(&dg00x->domain, &dg00x->rx_stream); }
static int pcm_capture_ack(struct snd_pcm_substream *substream) diff --git a/sound/firewire/fireface/ff-pcm.c b/sound/firewire/fireface/ff-pcm.c index 005d959f8651..5af1dce90921 100644 --- a/sound/firewire/fireface/ff-pcm.c +++ b/sound/firewire/fireface/ff-pcm.c @@ -341,14 +341,14 @@ static snd_pcm_uframes_t pcm_capture_pointer(struct snd_pcm_substream *sbstrm) { struct snd_ff *ff = sbstrm->private_data;
- return amdtp_stream_pcm_pointer(&ff->tx_stream); + return amdtp_domain_stream_pcm_pointer(&ff->domain, &ff->tx_stream); }
static snd_pcm_uframes_t pcm_playback_pointer(struct snd_pcm_substream *sbstrm) { struct snd_ff *ff = sbstrm->private_data;
- return amdtp_stream_pcm_pointer(&ff->rx_stream); + return amdtp_domain_stream_pcm_pointer(&ff->domain, &ff->rx_stream); }
static int pcm_capture_ack(struct snd_pcm_substream *substream) diff --git a/sound/firewire/fireworks/fireworks_pcm.c b/sound/firewire/fireworks/fireworks_pcm.c index abcc53dac8a5..71f5057caa0d 100644 --- a/sound/firewire/fireworks/fireworks_pcm.c +++ b/sound/firewire/fireworks/fireworks_pcm.c @@ -348,12 +348,14 @@ static int pcm_playback_trigger(struct snd_pcm_substream *substream, int cmd) static snd_pcm_uframes_t pcm_capture_pointer(struct snd_pcm_substream *sbstrm) { struct snd_efw *efw = sbstrm->private_data; - return amdtp_stream_pcm_pointer(&efw->tx_stream); + + return amdtp_domain_stream_pcm_pointer(&efw->domain, &efw->tx_stream); } static snd_pcm_uframes_t pcm_playback_pointer(struct snd_pcm_substream *sbstrm) { struct snd_efw *efw = sbstrm->private_data; - return amdtp_stream_pcm_pointer(&efw->rx_stream); + + return amdtp_domain_stream_pcm_pointer(&efw->domain, &efw->rx_stream); }
static int pcm_capture_ack(struct snd_pcm_substream *substream) diff --git a/sound/firewire/motu/motu-pcm.c b/sound/firewire/motu/motu-pcm.c index 00e693da0cad..13e2577c2a07 100644 --- a/sound/firewire/motu/motu-pcm.c +++ b/sound/firewire/motu/motu-pcm.c @@ -320,13 +320,13 @@ static snd_pcm_uframes_t capture_pointer(struct snd_pcm_substream *substream) { struct snd_motu *motu = substream->private_data;
- return amdtp_stream_pcm_pointer(&motu->tx_stream); + return amdtp_domain_stream_pcm_pointer(&motu->domain, &motu->tx_stream); } static snd_pcm_uframes_t playback_pointer(struct snd_pcm_substream *substream) { struct snd_motu *motu = substream->private_data;
- return amdtp_stream_pcm_pointer(&motu->rx_stream); + return amdtp_domain_stream_pcm_pointer(&motu->domain, &motu->rx_stream); }
static int capture_ack(struct snd_pcm_substream *substream) diff --git a/sound/firewire/oxfw/oxfw-pcm.c b/sound/firewire/oxfw/oxfw-pcm.c index ba586d1ac91d..3be35dfcf270 100644 --- a/sound/firewire/oxfw/oxfw-pcm.c +++ b/sound/firewire/oxfw/oxfw-pcm.c @@ -393,13 +393,13 @@ static snd_pcm_uframes_t pcm_capture_pointer(struct snd_pcm_substream *sbstm) { struct snd_oxfw *oxfw = sbstm->private_data;
- return amdtp_stream_pcm_pointer(&oxfw->tx_stream); + return amdtp_domain_stream_pcm_pointer(&oxfw->domain, &oxfw->tx_stream); } static snd_pcm_uframes_t pcm_playback_pointer(struct snd_pcm_substream *sbstm) { struct snd_oxfw *oxfw = sbstm->private_data;
- return amdtp_stream_pcm_pointer(&oxfw->rx_stream); + return amdtp_domain_stream_pcm_pointer(&oxfw->domain, &oxfw->rx_stream); }
static int pcm_capture_ack(struct snd_pcm_substream *substream) diff --git a/sound/firewire/tascam/tascam-pcm.c b/sound/firewire/tascam/tascam-pcm.c index b18664fdf955..1f66c8be7528 100644 --- a/sound/firewire/tascam/tascam-pcm.c +++ b/sound/firewire/tascam/tascam-pcm.c @@ -230,14 +230,14 @@ static snd_pcm_uframes_t pcm_capture_pointer(struct snd_pcm_substream *sbstrm) { struct snd_tscm *tscm = sbstrm->private_data;
- return amdtp_stream_pcm_pointer(&tscm->tx_stream); + return amdtp_domain_stream_pcm_pointer(&tscm->domain, &tscm->tx_stream); }
static snd_pcm_uframes_t pcm_playback_pointer(struct snd_pcm_substream *sbstrm) { struct snd_tscm *tscm = sbstrm->private_data;
- return amdtp_stream_pcm_pointer(&tscm->rx_stream); + return amdtp_domain_stream_pcm_pointer(&tscm->domain, &tscm->rx_stream); }
static int pcm_capture_ack(struct snd_pcm_substream *substream)
An isoc context for AMDTP stream is flushed to queue packet by a call of pcm.ack. This commit extends this for AMDTP domain.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/firewire/amdtp-stream.c | 24 +++++++++++++++--------- sound/firewire/amdtp-stream.h | 3 ++- sound/firewire/bebob/bebob_pcm.c | 4 ++-- sound/firewire/dice/dice-pcm.c | 4 ++-- sound/firewire/digi00x/digi00x-pcm.c | 4 ++-- sound/firewire/fireface/ff-pcm.c | 4 ++-- sound/firewire/fireworks/fireworks_pcm.c | 4 ++-- sound/firewire/motu/motu-pcm.c | 4 ++-- sound/firewire/oxfw/oxfw-pcm.c | 4 ++-- sound/firewire/tascam/tascam-pcm.c | 4 ++-- 10 files changed, 33 insertions(+), 26 deletions(-)
diff --git a/sound/firewire/amdtp-stream.c b/sound/firewire/amdtp-stream.c index 23677b805b05..3d27d4ce2b45 100644 --- a/sound/firewire/amdtp-stream.c +++ b/sound/firewire/amdtp-stream.c @@ -1139,23 +1139,29 @@ unsigned long amdtp_domain_stream_pcm_pointer(struct amdtp_domain *d, EXPORT_SYMBOL_GPL(amdtp_domain_stream_pcm_pointer);
/** - * amdtp_stream_pcm_ack - acknowledge queued PCM frames + * amdtp_domain_stream_pcm_ack - acknowledge queued PCM frames + * @d: the AMDTP domain. * @s: the AMDTP stream that transfers the PCM frames * * Returns zero always. */ -int amdtp_stream_pcm_ack(struct amdtp_stream *s) +int amdtp_domain_stream_pcm_ack(struct amdtp_domain *d, struct amdtp_stream *s) { - /* - * Process isochronous packets for recent isochronous cycle to handle - * queued PCM frames. - */ - if (amdtp_stream_running(s)) - fw_iso_context_flush_completions(s->context); + struct amdtp_stream *irq_target = d->irq_target; + + // Process isochronous packets for recent isochronous cycle to handle + // queued PCM frames. + if (irq_target && amdtp_stream_running(irq_target)) { + // Queued packet should be processed without any kernel + // preemption to keep latency against bus cycle. + preempt_disable(); + fw_iso_context_flush_completions(irq_target->context); + preempt_enable(); + }
return 0; } -EXPORT_SYMBOL(amdtp_stream_pcm_ack); +EXPORT_SYMBOL_GPL(amdtp_domain_stream_pcm_ack);
/** * amdtp_stream_update - update the stream after a bus reset diff --git a/sound/firewire/amdtp-stream.h b/sound/firewire/amdtp-stream.h index ba0bbeddfdcb..470e77ca0061 100644 --- a/sound/firewire/amdtp-stream.h +++ b/sound/firewire/amdtp-stream.h @@ -198,7 +198,6 @@ int amdtp_stream_add_pcm_hw_constraints(struct amdtp_stream *s, struct snd_pcm_runtime *runtime);
void amdtp_stream_pcm_prepare(struct amdtp_stream *s); -int amdtp_stream_pcm_ack(struct amdtp_stream *s); void amdtp_stream_pcm_abort(struct amdtp_stream *s);
extern const unsigned int amdtp_syt_intervals[CIP_SFC_COUNT]; @@ -303,4 +302,6 @@ static inline int amdtp_domain_set_events_per_period(struct amdtp_domain *d,
unsigned long amdtp_domain_stream_pcm_pointer(struct amdtp_domain *d, struct amdtp_stream *s); +int amdtp_domain_stream_pcm_ack(struct amdtp_domain *d, struct amdtp_stream *s); + #endif diff --git a/sound/firewire/bebob/bebob_pcm.c b/sound/firewire/bebob/bebob_pcm.c index dc15ea8d0dc5..1b100159f4c5 100644 --- a/sound/firewire/bebob/bebob_pcm.c +++ b/sound/firewire/bebob/bebob_pcm.c @@ -332,14 +332,14 @@ static int pcm_capture_ack(struct snd_pcm_substream *substream) { struct snd_bebob *bebob = substream->private_data;
- return amdtp_stream_pcm_ack(&bebob->tx_stream); + return amdtp_domain_stream_pcm_ack(&bebob->domain, &bebob->tx_stream); }
static int pcm_playback_ack(struct snd_pcm_substream *substream) { struct snd_bebob *bebob = substream->private_data;
- return amdtp_stream_pcm_ack(&bebob->rx_stream); + return amdtp_domain_stream_pcm_ack(&bebob->domain, &bebob->rx_stream); }
int snd_bebob_create_pcm_devices(struct snd_bebob *bebob) diff --git a/sound/firewire/dice/dice-pcm.c b/sound/firewire/dice/dice-pcm.c index 81b78e7d181d..f1848fb39bd0 100644 --- a/sound/firewire/dice/dice-pcm.c +++ b/sound/firewire/dice/dice-pcm.c @@ -394,7 +394,7 @@ static int capture_ack(struct snd_pcm_substream *substream) struct snd_dice *dice = substream->private_data; struct amdtp_stream *stream = &dice->tx_stream[substream->pcm->device];
- return amdtp_stream_pcm_ack(stream); + return amdtp_domain_stream_pcm_ack(&dice->domain, stream); }
static int playback_ack(struct snd_pcm_substream *substream) @@ -402,7 +402,7 @@ static int playback_ack(struct snd_pcm_substream *substream) struct snd_dice *dice = substream->private_data; struct amdtp_stream *stream = &dice->rx_stream[substream->pcm->device];
- return amdtp_stream_pcm_ack(stream); + return amdtp_domain_stream_pcm_ack(&dice->domain, stream); }
int snd_dice_create_pcm(struct snd_dice *dice) diff --git a/sound/firewire/digi00x/digi00x-pcm.c b/sound/firewire/digi00x/digi00x-pcm.c index f6a2053d5f10..8befc5d2ef22 100644 --- a/sound/firewire/digi00x/digi00x-pcm.c +++ b/sound/firewire/digi00x/digi00x-pcm.c @@ -315,14 +315,14 @@ static int pcm_capture_ack(struct snd_pcm_substream *substream) { struct snd_dg00x *dg00x = substream->private_data;
- return amdtp_stream_pcm_ack(&dg00x->tx_stream); + return amdtp_domain_stream_pcm_ack(&dg00x->domain, &dg00x->tx_stream); }
static int pcm_playback_ack(struct snd_pcm_substream *substream) { struct snd_dg00x *dg00x = substream->private_data;
- return amdtp_stream_pcm_ack(&dg00x->rx_stream); + return amdtp_domain_stream_pcm_ack(&dg00x->domain, &dg00x->rx_stream); }
int snd_dg00x_create_pcm_devices(struct snd_dg00x *dg00x) diff --git a/sound/firewire/fireface/ff-pcm.c b/sound/firewire/fireface/ff-pcm.c index 5af1dce90921..c29f87a65c0f 100644 --- a/sound/firewire/fireface/ff-pcm.c +++ b/sound/firewire/fireface/ff-pcm.c @@ -355,14 +355,14 @@ static int pcm_capture_ack(struct snd_pcm_substream *substream) { struct snd_ff *ff = substream->private_data;
- return amdtp_stream_pcm_ack(&ff->tx_stream); + return amdtp_domain_stream_pcm_ack(&ff->domain, &ff->tx_stream); }
static int pcm_playback_ack(struct snd_pcm_substream *substream) { struct snd_ff *ff = substream->private_data;
- return amdtp_stream_pcm_ack(&ff->rx_stream); + return amdtp_domain_stream_pcm_ack(&ff->domain, &ff->rx_stream); }
int snd_ff_create_pcm_devices(struct snd_ff *ff) diff --git a/sound/firewire/fireworks/fireworks_pcm.c b/sound/firewire/fireworks/fireworks_pcm.c index 71f5057caa0d..64c1bcf28dfa 100644 --- a/sound/firewire/fireworks/fireworks_pcm.c +++ b/sound/firewire/fireworks/fireworks_pcm.c @@ -362,14 +362,14 @@ static int pcm_capture_ack(struct snd_pcm_substream *substream) { struct snd_efw *efw = substream->private_data;
- return amdtp_stream_pcm_ack(&efw->tx_stream); + return amdtp_domain_stream_pcm_ack(&efw->domain, &efw->tx_stream); }
static int pcm_playback_ack(struct snd_pcm_substream *substream) { struct snd_efw *efw = substream->private_data;
- return amdtp_stream_pcm_ack(&efw->rx_stream); + return amdtp_domain_stream_pcm_ack(&efw->domain, &efw->rx_stream); }
int snd_efw_create_pcm_devices(struct snd_efw *efw) diff --git a/sound/firewire/motu/motu-pcm.c b/sound/firewire/motu/motu-pcm.c index 13e2577c2a07..55d3d6661731 100644 --- a/sound/firewire/motu/motu-pcm.c +++ b/sound/firewire/motu/motu-pcm.c @@ -333,14 +333,14 @@ static int capture_ack(struct snd_pcm_substream *substream) { struct snd_motu *motu = substream->private_data;
- return amdtp_stream_pcm_ack(&motu->tx_stream); + return amdtp_domain_stream_pcm_ack(&motu->domain, &motu->tx_stream); }
static int playback_ack(struct snd_pcm_substream *substream) { struct snd_motu *motu = substream->private_data;
- return amdtp_stream_pcm_ack(&motu->rx_stream); + return amdtp_domain_stream_pcm_ack(&motu->domain, &motu->rx_stream); }
int snd_motu_create_pcm_devices(struct snd_motu *motu) diff --git a/sound/firewire/oxfw/oxfw-pcm.c b/sound/firewire/oxfw/oxfw-pcm.c index 3be35dfcf270..74bd1811cec2 100644 --- a/sound/firewire/oxfw/oxfw-pcm.c +++ b/sound/firewire/oxfw/oxfw-pcm.c @@ -406,14 +406,14 @@ static int pcm_capture_ack(struct snd_pcm_substream *substream) { struct snd_oxfw *oxfw = substream->private_data;
- return amdtp_stream_pcm_ack(&oxfw->tx_stream); + return amdtp_domain_stream_pcm_ack(&oxfw->domain, &oxfw->tx_stream); }
static int pcm_playback_ack(struct snd_pcm_substream *substream) { struct snd_oxfw *oxfw = substream->private_data;
- return amdtp_stream_pcm_ack(&oxfw->rx_stream); + return amdtp_domain_stream_pcm_ack(&oxfw->domain, &oxfw->rx_stream); }
int snd_oxfw_create_pcm(struct snd_oxfw *oxfw) diff --git a/sound/firewire/tascam/tascam-pcm.c b/sound/firewire/tascam/tascam-pcm.c index 1f66c8be7528..cd45f20ba515 100644 --- a/sound/firewire/tascam/tascam-pcm.c +++ b/sound/firewire/tascam/tascam-pcm.c @@ -244,14 +244,14 @@ static int pcm_capture_ack(struct snd_pcm_substream *substream) { struct snd_tscm *tscm = substream->private_data;
- return amdtp_stream_pcm_ack(&tscm->tx_stream); + return amdtp_domain_stream_pcm_ack(&tscm->domain, &tscm->tx_stream); }
static int pcm_playback_ack(struct snd_pcm_substream *substream) { struct snd_tscm *tscm = substream->private_data;
- return amdtp_stream_pcm_ack(&tscm->rx_stream); + return amdtp_domain_stream_pcm_ack(&tscm->domain, &tscm->rx_stream); }
int snd_tscm_create_pcm_devices(struct snd_tscm *tscm)
The aim of AMDTP domain is to process several isoc context in the same time. However, current implementation is against this idea because it flushes each isoc context in the end of processing context callback.
This commit cancels it.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/firewire/amdtp-stream.c | 4 ---- 1 file changed, 4 deletions(-)
diff --git a/sound/firewire/amdtp-stream.c b/sound/firewire/amdtp-stream.c index 3d27d4ce2b45..36c3f1f9dbff 100644 --- a/sound/firewire/amdtp-stream.c +++ b/sound/firewire/amdtp-stream.c @@ -842,8 +842,6 @@ static void out_stream_callback(struct fw_iso_context *context, u32 tstamp, }
s->event_count = event_count; - - fw_iso_context_queue_flush(s->context); }
static void in_stream_callback(struct fw_iso_context *context, u32 tstamp, @@ -897,8 +895,6 @@ static void in_stream_callback(struct fw_iso_context *context, u32 tstamp, }
s->event_count = event_count; - - fw_iso_context_queue_flush(s->context); }
/* this is executed one time */
This commit changes AMDTP domain to run on an IT context of 1394 OHCI as IRQ target. No hardware interrupt is scheduled for the other isoc contexts. All of the isoc context are processed in a callback for an isoc context of IRQ target.
The IRQ target is automatically selected from a list of AMDTP streams, thus users of AMDTP domain should add an AMDTP stream for IT context at least.
The reason to select IT context as IRQ target is that the IT context runs on local 1394 OHCI controller and it can be used as reliable, constant IRQ generator. On the other hand, IR context can include skip cycle according to isoc packet transferred by device.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/firewire/amdtp-stream.c | 179 ++++++++++++++++++++++++++-------- sound/firewire/amdtp-stream.h | 7 +- 2 files changed, 140 insertions(+), 46 deletions(-)
diff --git a/sound/firewire/amdtp-stream.c b/sound/firewire/amdtp-stream.c index 36c3f1f9dbff..48be31eae9a5 100644 --- a/sound/firewire/amdtp-stream.c +++ b/sound/firewire/amdtp-stream.c @@ -482,13 +482,13 @@ static inline int queue_out_packet(struct amdtp_stream *s, }
static inline int queue_in_packet(struct amdtp_stream *s, - struct fw_iso_packet *params, bool sched_irq) + struct fw_iso_packet *params) { // Queue one packet for IR context. params->header_length = s->ctx_data.tx.ctx_header_size; params->payload_length = s->ctx_data.tx.max_ctx_payload_length; params->skip = false; - return queue_packet(s, params, sched_irq); + return queue_packet(s, params, false); }
static void generate_cip_header(struct amdtp_stream *s, __be32 cip_header[2], @@ -790,15 +790,24 @@ static void process_ctx_payloads(struct amdtp_stream *s, update_pcm_pointers(s, pcm, pcm_frames); }
+static void amdtp_stream_master_callback(struct fw_iso_context *context, + u32 tstamp, size_t header_length, + void *header, void *private_data); + +static void amdtp_stream_master_first_callback(struct fw_iso_context *context, + u32 tstamp, size_t header_length, + void *header, void *private_data); + static void out_stream_callback(struct fw_iso_context *context, u32 tstamp, size_t header_length, void *header, void *private_data) { struct amdtp_stream *s = private_data; const __be32 *ctx_header = header; - unsigned int events_per_period = s->events_per_period; - unsigned int event_count = s->event_count; + unsigned int events_per_period = s->ctx_data.rx.events_per_period; + unsigned int event_count = s->ctx_data.rx.event_count; unsigned int packets; + bool is_irq_target; int i;
if (s->packet_index < 0) @@ -811,6 +820,10 @@ static void out_stream_callback(struct fw_iso_context *context, u32 tstamp,
process_ctx_payloads(s, s->pkt_descs, packets);
+ is_irq_target = + !!(context->callback.sc == amdtp_stream_master_callback || + context->callback.sc == amdtp_stream_master_first_callback); + for (i = 0; i < packets; ++i) { const struct pkt_desc *desc = s->pkt_descs + i; unsigned int syt; @@ -829,10 +842,12 @@ static void out_stream_callback(struct fw_iso_context *context, u32 tstamp, desc->data_blocks, desc->data_block_counter, syt, i);
- event_count += desc->data_blocks; - if (event_count >= events_per_period) { - event_count -= events_per_period; - sched_irq = true; + if (is_irq_target) { + event_count += desc->data_blocks; + if (event_count >= events_per_period) { + event_count -= events_per_period; + sched_irq = true; + } }
if (queue_out_packet(s, &template.params, sched_irq) < 0) { @@ -841,7 +856,7 @@ static void out_stream_callback(struct fw_iso_context *context, u32 tstamp, } }
- s->event_count = event_count; + s->ctx_data.rx.event_count = event_count; }
static void in_stream_callback(struct fw_iso_context *context, u32 tstamp, @@ -850,8 +865,6 @@ static void in_stream_callback(struct fw_iso_context *context, u32 tstamp, { struct amdtp_stream *s = private_data; __be32 *ctx_header = header; - unsigned int events_per_period = s->events_per_period; - unsigned int event_count = s->event_count; unsigned int packets; int i; int err; @@ -873,31 +886,47 @@ static void in_stream_callback(struct fw_iso_context *context, u32 tstamp, }
for (i = 0; i < packets; ++i) { - const struct pkt_desc *desc = s->pkt_descs + i; struct fw_iso_packet params = {0}; - bool sched_irq = false; - - if (err >= 0) { - event_count += desc->data_blocks; - if (event_count >= events_per_period) { - event_count -= events_per_period; - sched_irq = true; - } - } else { - sched_irq = - !((s->packet_index + 1) % s->idle_irq_interval); - }
- if (queue_in_packet(s, ¶ms, sched_irq) < 0) { + if (queue_in_packet(s, ¶ms) < 0) { cancel_stream(s); return; } } +} + +static void amdtp_stream_master_callback(struct fw_iso_context *context, + u32 tstamp, size_t header_length, + void *header, void *private_data) +{ + struct amdtp_domain *d = private_data; + struct amdtp_stream *irq_target = d->irq_target; + struct amdtp_stream *s; + + out_stream_callback(context, tstamp, header_length, header, irq_target); + if (amdtp_streaming_error(irq_target)) + goto error;
- s->event_count = event_count; + list_for_each_entry(s, &d->streams, list) { + if (s != irq_target && amdtp_stream_running(s)) { + fw_iso_context_flush_completions(s->context); + if (amdtp_streaming_error(s)) + goto error; + } + } + + return; +error: + if (amdtp_stream_running(irq_target)) + cancel_stream(irq_target); + + list_for_each_entry(s, &d->streams, list) { + if (amdtp_stream_running(s)) + cancel_stream(s); + } }
-/* this is executed one time */ +// this is executed one time. static void amdtp_stream_first_callback(struct fw_iso_context *context, u32 tstamp, size_t header_length, void *header, void *private_data) @@ -928,18 +957,39 @@ static void amdtp_stream_first_callback(struct fw_iso_context *context, context->callback.sc(context, tstamp, header_length, header, s); }
+static void amdtp_stream_master_first_callback(struct fw_iso_context *context, + u32 tstamp, size_t header_length, + void *header, void *private_data) +{ + struct amdtp_domain *d = private_data; + struct amdtp_stream *s = d->irq_target; + const __be32 *ctx_header = header; + + s->callbacked = true; + wake_up(&s->callback_wait); + + s->start_cycle = compute_it_cycle(*ctx_header, s->queue_size); + + context->callback.sc = amdtp_stream_master_callback; + + context->callback.sc(context, tstamp, header_length, header, d); +} + /** * amdtp_stream_start - start transferring packets * @s: the AMDTP stream to start * @channel: the isochronous channel on the bus * @speed: firewire speed code + * @d: the AMDTP domain to which the AMDTP stream belongs + * @is_irq_target: whether isoc context for the AMDTP stream is used to generate + * hardware IRQ. * * The stream cannot be started until it has been configured with * amdtp_stream_set_parameters() and it must be started before any PCM or MIDI * device can be started. */ static int amdtp_stream_start(struct amdtp_stream *s, int channel, int speed, - struct amdtp_domain *d) + struct amdtp_domain *d, bool is_irq_target) { static const struct { unsigned int data_block; @@ -955,10 +1005,13 @@ static int amdtp_stream_start(struct amdtp_stream *s, int channel, int speed, }; unsigned int events_per_buffer = d->events_per_buffer; unsigned int events_per_period = d->events_per_period; + unsigned int idle_irq_interval; unsigned int ctx_header_size; unsigned int max_ctx_payload_size; enum dma_data_direction dir; int type, tag, err; + fw_iso_callback_t ctx_cb; + void *ctx_data;
mutex_lock(&s->mutex);
@@ -969,6 +1022,12 @@ static int amdtp_stream_start(struct amdtp_stream *s, int channel, int speed, }
if (s->direction == AMDTP_IN_STREAM) { + // NOTE: IT context should be used for constant IRQ. + if (is_irq_target) { + err = -EINVAL; + goto err_unlock; + } + s->data_block_counter = UINT_MAX; } else { entry = &initial_state[s->sfc]; @@ -1008,22 +1067,29 @@ static int amdtp_stream_start(struct amdtp_stream *s, int channel, int speed, if (events_per_buffer == 0) events_per_buffer = events_per_period * 3;
- s->idle_irq_interval = - DIV_ROUND_UP(CYCLES_PER_SECOND * events_per_period, - amdtp_rate_table[s->sfc]); + idle_irq_interval = DIV_ROUND_UP(CYCLES_PER_SECOND * events_per_period, + amdtp_rate_table[s->sfc]); s->queue_size = DIV_ROUND_UP(CYCLES_PER_SECOND * events_per_buffer, amdtp_rate_table[s->sfc]); - s->events_per_period = events_per_period; - s->event_count = 0;
err = iso_packets_buffer_init(&s->buffer, s->unit, s->queue_size, max_ctx_payload_size, dir); if (err < 0) goto err_unlock;
+ if (is_irq_target) { + s->ctx_data.rx.events_per_period = events_per_period; + s->ctx_data.rx.event_count = 0; + ctx_cb = amdtp_stream_master_first_callback; + ctx_data = d; + } else { + ctx_cb = amdtp_stream_first_callback; + ctx_data = s; + } + s->context = fw_iso_context_create(fw_parent_device(s->unit)->card, type, channel, speed, ctx_header_size, - amdtp_stream_first_callback, s); + ctx_cb, ctx_data); if (IS_ERR(s->context)) { err = PTR_ERR(s->context); if (err == -EBUSY) @@ -1054,14 +1120,20 @@ static int amdtp_stream_start(struct amdtp_stream *s, int channel, int speed, s->packet_index = 0; do { struct fw_iso_packet params; - bool sched_irq;
- sched_irq = !((s->packet_index + 1) % s->idle_irq_interval); if (s->direction == AMDTP_IN_STREAM) { - err = queue_in_packet(s, ¶ms, sched_irq); + err = queue_in_packet(s, ¶ms); } else { + bool sched_irq = false; + params.header_length = 0; params.payload_length = 0; + + if (is_irq_target) { + sched_irq = !((s->packet_index + 1) % + idle_irq_interval); + } + err = queue_out_packet(s, ¶ms, sched_irq); } if (err < 0) @@ -1276,17 +1348,33 @@ int amdtp_domain_start(struct amdtp_domain *d) struct amdtp_stream *s; int err = 0;
+ // Select an IT context as IRQ target. list_for_each_entry(s, &d->streams, list) { - err = amdtp_stream_start(s, s->channel, s->speed, d); - if (err < 0) + if (s->direction == AMDTP_OUT_STREAM) break; } + if (!s) + return -ENXIO; + d->irq_target = s;
- if (err < 0) { - list_for_each_entry(s, &d->streams, list) - amdtp_stream_stop(s); + list_for_each_entry(s, &d->streams, list) { + if (s != d->irq_target) { + err = amdtp_stream_start(s, s->channel, s->speed, d, + false); + if (err < 0) + goto error; + } }
+ s = d->irq_target; + err = amdtp_stream_start(s, s->channel, s->speed, d, true); + if (err < 0) + goto error; + + return 0; +error: + list_for_each_entry(s, &d->streams, list) + amdtp_stream_stop(s); return err; } EXPORT_SYMBOL_GPL(amdtp_domain_start); @@ -1299,12 +1387,17 @@ void amdtp_domain_stop(struct amdtp_domain *d) { struct amdtp_stream *s, *next;
+ if (d->irq_target) + amdtp_stream_stop(d->irq_target); + list_for_each_entry_safe(s, next, &d->streams, list) { list_del(&s->list);
- amdtp_stream_stop(s); + if (s != d->irq_target) + amdtp_stream_stop(s); }
d->events_per_period = 0; + d->irq_target = NULL; } EXPORT_SYMBOL_GPL(amdtp_domain_stop); diff --git a/sound/firewire/amdtp-stream.h b/sound/firewire/amdtp-stream.h index 470e77ca0061..c4bde69c2d4f 100644 --- a/sound/firewire/amdtp-stream.h +++ b/sound/firewire/amdtp-stream.h @@ -143,11 +143,12 @@ struct amdtp_stream { // To generate CIP header. unsigned int fdf; int syt_override; + + // To generate constant hardware IRQ. + unsigned int event_count; + unsigned int events_per_period; } rx; } ctx_data; - unsigned int event_count; - unsigned int events_per_period; - unsigned int idle_irq_interval;
/* For CIP headers. */ unsigned int source_node_id_field;
Some devices have a quirk to postpone transmission of isoc packet for several dozen or hundred isoc cycles since configured to transmit. Furthermore, some devices have a quirk to transmit isoc packet with discontinued data of its header.
In 1394 OHCI specification, software allows to start isoc context with certain isoc cycle. Linux firewire subsystem has kernel API to use it as well.
This commit uses the functionality of 1394 OHCI controller to handle the quirks. At present, this feature is convenient to ALSA bebob and fireface driver. As a result, some devices can be safely handled, as long as I know: - MAudio FireWire solo - MAudio ProFire Lightbridge - MAudio FireWire 410 - Roland FA-66
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/firewire/amdtp-stream.c | 79 +++++++++++++++++++-- sound/firewire/amdtp-stream.h | 2 +- sound/firewire/bebob/bebob_stream.c | 10 ++- sound/firewire/dice/dice-stream.c | 2 +- sound/firewire/digi00x/digi00x-stream.c | 2 +- sound/firewire/fireface/ff-stream.c | 10 ++- sound/firewire/fireworks/fireworks_stream.c | 2 +- sound/firewire/motu/motu-stream.c | 2 +- sound/firewire/oxfw/oxfw-stream.c | 2 +- sound/firewire/tascam/tascam-stream.c | 2 +- 10 files changed, 98 insertions(+), 15 deletions(-)
diff --git a/sound/firewire/amdtp-stream.c b/sound/firewire/amdtp-stream.c index 48be31eae9a5..37d38efb4c87 100644 --- a/sound/firewire/amdtp-stream.c +++ b/sound/firewire/amdtp-stream.c @@ -9,6 +9,7 @@ #include <linux/device.h> #include <linux/err.h> #include <linux/firewire.h> +#include <linux/firewire-constants.h> #include <linux/module.h> #include <linux/slab.h> #include <sound/pcm.h> @@ -983,13 +984,16 @@ static void amdtp_stream_master_first_callback(struct fw_iso_context *context, * @d: the AMDTP domain to which the AMDTP stream belongs * @is_irq_target: whether isoc context for the AMDTP stream is used to generate * hardware IRQ. + * @start_cycle: the isochronous cycle to start the context. Start immediately + * if negative value is given. * * The stream cannot be started until it has been configured with * amdtp_stream_set_parameters() and it must be started before any PCM or MIDI * device can be started. */ static int amdtp_stream_start(struct amdtp_stream *s, int channel, int speed, - struct amdtp_domain *d, bool is_irq_target) + struct amdtp_domain *d, bool is_irq_target, + int start_cycle) { static const struct { unsigned int data_block; @@ -1146,7 +1150,7 @@ static int amdtp_stream_start(struct amdtp_stream *s, int channel, int speed, tag |= FW_ISO_CONTEXT_MATCH_TAG0;
s->callbacked = false; - err = fw_iso_context_start(s->context, -1, 0, tag); + err = fw_iso_context_start(s->context, start_cycle, 0, tag); if (err < 0) goto err_pkt_descs;
@@ -1339,14 +1343,40 @@ int amdtp_domain_add_stream(struct amdtp_domain *d, struct amdtp_stream *s, } EXPORT_SYMBOL_GPL(amdtp_domain_add_stream);
+static int get_current_cycle_time(struct fw_card *fw_card, int *cur_cycle) +{ + int generation; + int rcode; + __be32 reg; + u32 data; + + // This is a request to local 1394 OHCI controller and expected to + // complete without any event waiting. + generation = fw_card->generation; + smp_rmb(); // node_id vs. generation. + rcode = fw_run_transaction(fw_card, TCODE_READ_QUADLET_REQUEST, + fw_card->node_id, generation, SCODE_100, + CSR_REGISTER_BASE + CSR_CYCLE_TIME, + ®, sizeof(reg)); + if (rcode != RCODE_COMPLETE) + return -EIO; + + data = be32_to_cpu(reg); + *cur_cycle = data >> 12; + + return 0; +} + /** * amdtp_domain_start - start sending packets for isoc context in the domain. * @d: the AMDTP domain. + * @ir_delay_cycle: the cycle delay to start all IR contexts. */ -int amdtp_domain_start(struct amdtp_domain *d) +int amdtp_domain_start(struct amdtp_domain *d, unsigned int ir_delay_cycle) { struct amdtp_stream *s; - int err = 0; + int cycle; + int err;
// Select an IT context as IRQ target. list_for_each_entry(s, &d->streams, list) { @@ -1357,17 +1387,54 @@ int amdtp_domain_start(struct amdtp_domain *d) return -ENXIO; d->irq_target = s;
+ if (ir_delay_cycle > 0) { + struct fw_card *fw_card = fw_parent_device(s->unit)->card; + + err = get_current_cycle_time(fw_card, &cycle); + if (err < 0) + return err; + + // No need to care overflow in cycle field because of enough + // width. + cycle += ir_delay_cycle; + + // Round up to sec field. + if ((cycle & 0x00001fff) >= CYCLES_PER_SECOND) { + unsigned int sec; + + // The sec field can overflow. + sec = (cycle & 0xffffe000) >> 13; + cycle = (++sec << 13) | + ((cycle & 0x00001fff) / CYCLES_PER_SECOND); + } + + // In OHCI 1394 specification, lower 2 bits are available for + // sec field. + cycle &= 0x00007fff; + } else { + cycle = -1; + } + list_for_each_entry(s, &d->streams, list) { + int cycle_match; + + if (s->direction == AMDTP_IN_STREAM) { + cycle_match = cycle; + } else { + // IT context starts immediately. + cycle_match = -1; + } + if (s != d->irq_target) { err = amdtp_stream_start(s, s->channel, s->speed, d, - false); + false, cycle_match); if (err < 0) goto error; } }
s = d->irq_target; - err = amdtp_stream_start(s, s->channel, s->speed, d, true); + err = amdtp_stream_start(s, s->channel, s->speed, d, true, -1); if (err < 0) goto error;
diff --git a/sound/firewire/amdtp-stream.h b/sound/firewire/amdtp-stream.h index c4bde69c2d4f..f2d44e2dc3c8 100644 --- a/sound/firewire/amdtp-stream.h +++ b/sound/firewire/amdtp-stream.h @@ -288,7 +288,7 @@ void amdtp_domain_destroy(struct amdtp_domain *d); int amdtp_domain_add_stream(struct amdtp_domain *d, struct amdtp_stream *s, int channel, int speed);
-int amdtp_domain_start(struct amdtp_domain *d); +int amdtp_domain_start(struct amdtp_domain *d, unsigned int ir_delay_cycle); void amdtp_domain_stop(struct amdtp_domain *d);
static inline int amdtp_domain_set_events_per_period(struct amdtp_domain *d, diff --git a/sound/firewire/bebob/bebob_stream.c b/sound/firewire/bebob/bebob_stream.c index 5e4a61458be2..7ac0d9f495c4 100644 --- a/sound/firewire/bebob/bebob_stream.c +++ b/sound/firewire/bebob/bebob_stream.c @@ -658,7 +658,15 @@ int snd_bebob_stream_start_duplex(struct snd_bebob *bebob) if (err < 0) goto error;
- err = amdtp_domain_start(&bebob->domain); + // The device postpones start of transmission mostly for 1 sec + // after receives packets firstly. For safe, IR context starts + // 1.5 sec (=12000 cycles) later. This is within 2.0 sec + // (=CALLBACK_TIMEOUT). + // Furthermore, some devices transfer isoc packets with + // discontinuous counter in the beginning of packet streaming. + // The delay has an effect to avoid detection of this + // discontinuity. + err = amdtp_domain_start(&bebob->domain, 12000); if (err < 0) goto error;
diff --git a/sound/firewire/dice/dice-stream.c b/sound/firewire/dice/dice-stream.c index 0cff346e8052..6a3d60913e10 100644 --- a/sound/firewire/dice/dice-stream.c +++ b/sound/firewire/dice/dice-stream.c @@ -462,7 +462,7 @@ int snd_dice_stream_start_duplex(struct snd_dice *dice) goto error; }
- err = amdtp_domain_start(&dice->domain); + err = amdtp_domain_start(&dice->domain, 0); if (err < 0) goto error;
diff --git a/sound/firewire/digi00x/digi00x-stream.c b/sound/firewire/digi00x/digi00x-stream.c index 0c539188ba18..405d6903bfbc 100644 --- a/sound/firewire/digi00x/digi00x-stream.c +++ b/sound/firewire/digi00x/digi00x-stream.c @@ -375,7 +375,7 @@ int snd_dg00x_stream_start_duplex(struct snd_dg00x *dg00x) if (err < 0) goto error;
- err = amdtp_domain_start(&dg00x->domain); + err = amdtp_domain_start(&dg00x->domain, 0); if (err < 0) goto error;
diff --git a/sound/firewire/fireface/ff-stream.c b/sound/firewire/fireface/ff-stream.c index a13754f914e8..63b79c4a5405 100644 --- a/sound/firewire/fireface/ff-stream.c +++ b/sound/firewire/fireface/ff-stream.c @@ -184,6 +184,7 @@ int snd_ff_stream_start_duplex(struct snd_ff *ff, unsigned int rate) */ if (!amdtp_stream_running(&ff->rx_stream)) { int spd = fw_parent_device(ff->unit)->max_speed; + unsigned int ir_delay_cycle;
err = ff->spec->protocol->begin_session(ff, rate); if (err < 0) @@ -199,7 +200,14 @@ int snd_ff_stream_start_duplex(struct snd_ff *ff, unsigned int rate) if (err < 0) goto error;
- err = amdtp_domain_start(&ff->domain); + // The device postpones start of transmission mostly for several + // cycles after receiving packets firstly. + if (ff->spec->protocol == &snd_ff_protocol_ff800) + ir_delay_cycle = 800; // = 100 msec + else + ir_delay_cycle = 16; // = 2 msec + + err = amdtp_domain_start(&ff->domain, ir_delay_cycle); if (err < 0) goto error;
diff --git a/sound/firewire/fireworks/fireworks_stream.c b/sound/firewire/fireworks/fireworks_stream.c index f35a33d4d4e6..2206af0fef42 100644 --- a/sound/firewire/fireworks/fireworks_stream.c +++ b/sound/firewire/fireworks/fireworks_stream.c @@ -272,7 +272,7 @@ int snd_efw_stream_start_duplex(struct snd_efw *efw) if (err < 0) goto error;
- err = amdtp_domain_start(&efw->domain); + err = amdtp_domain_start(&efw->domain, 0); if (err < 0) goto error;
diff --git a/sound/firewire/motu/motu-stream.c b/sound/firewire/motu/motu-stream.c index 9975770c9b1f..a17ddceb1bec 100644 --- a/sound/firewire/motu/motu-stream.c +++ b/sound/firewire/motu/motu-stream.c @@ -260,7 +260,7 @@ int snd_motu_stream_start_duplex(struct snd_motu *motu) if (err < 0) goto stop_streams;
- err = amdtp_domain_start(&motu->domain); + err = amdtp_domain_start(&motu->domain, 0); if (err < 0) goto stop_streams;
diff --git a/sound/firewire/oxfw/oxfw-stream.c b/sound/firewire/oxfw/oxfw-stream.c index 995e9c5bd84b..501a80094bf7 100644 --- a/sound/firewire/oxfw/oxfw-stream.c +++ b/sound/firewire/oxfw/oxfw-stream.c @@ -355,7 +355,7 @@ int snd_oxfw_stream_start_duplex(struct snd_oxfw *oxfw) } }
- err = amdtp_domain_start(&oxfw->domain); + err = amdtp_domain_start(&oxfw->domain, 0); if (err < 0) goto error;
diff --git a/sound/firewire/tascam/tascam-stream.c b/sound/firewire/tascam/tascam-stream.c index a9b3b7eb6d21..eb07e1decf9b 100644 --- a/sound/firewire/tascam/tascam-stream.c +++ b/sound/firewire/tascam/tascam-stream.c @@ -473,7 +473,7 @@ int snd_tscm_stream_start_duplex(struct snd_tscm *tscm, unsigned int rate) if (err < 0) goto error;
- err = amdtp_domain_start(&tscm->domain); + err = amdtp_domain_start(&tscm->domain, 0); if (err < 0) return err;
On Fri, 18 Oct 2019 08:19:05 +0200, Takashi Sakamoto wrote:
Hi,
This patchset is a part of my continuous work to improve ALSA IEC 61883-1/6 packet streaming engine for clock-recovery, addressed in my previous message: https://mailman.alsa-project.org/pipermail/alsa-devel/2019-August/153388.htm...
For the clock-recovery, information in the sequence of tx packet from device should be used to generate the sequence of rx packet to the device. In current implementation of the engine, the packets are processed in different tasklet contexts for each IR/IT context. This is inconvenient to bypass information between these IR/IT contexts.
In this patchset, the engine is improved to process all of IR/IT contexts in the same domain by a tasklet context for one of IT context. For convenience, the IT context is called as 'IRQ target'. As a result, 1394 OHCI controller is managed to generate hardware IRQ just for the IRQ target. All of rx/tx packets are processed in tasklet for the hardware IRQ.
Takashi Sakamoto (6): ALSA: firewire-lib: add irq_target member into amdtp_domain struct ALSA: firewire-lib: replace pointer callback to flush isoc contexts in AMDTP domain ALSA: firewire-lib: replace ack callback to flush isoc contexts in AMDTP domain ALSA: firewire-lib: cancel flushing isoc context in the laste step to process context callback ALSA: firewire-lib: handle several AMDTP streams in callback handler of IRQ target ALSA: firewire-lib: postpone to start IR context
Applied all patches now.
Although the preempt handling in AMDTP looks a bit suspicious, it should be OK as long as the code has been tested, so I took as is.
thanks,
Takashi
On Sat, Oct 19, 2019 at 09:22:16AM +0200, Takashi Iwai wrote:
On Fri, 18 Oct 2019 08:19:05 +0200, Takashi Sakamoto wrote:
Hi,
This patchset is a part of my continuous work to improve ALSA IEC 61883-1/6 packet streaming engine for clock-recovery, addressed in my previous message: https://mailman.alsa-project.org/pipermail/alsa-devel/2019-August/153388.htm...
For the clock-recovery, information in the sequence of tx packet from device should be used to generate the sequence of rx packet to the device. In current implementation of the engine, the packets are processed in different tasklet contexts for each IR/IT context. This is inconvenient to bypass information between these IR/IT contexts.
In this patchset, the engine is improved to process all of IR/IT contexts in the same domain by a tasklet context for one of IT context. For convenience, the IT context is called as 'IRQ target'. As a result, 1394 OHCI controller is managed to generate hardware IRQ just for the IRQ target. All of rx/tx packets are processed in tasklet for the hardware IRQ.
Takashi Sakamoto (6): ALSA: firewire-lib: add irq_target member into amdtp_domain struct ALSA: firewire-lib: replace pointer callback to flush isoc contexts in AMDTP domain ALSA: firewire-lib: replace ack callback to flush isoc contexts in AMDTP domain ALSA: firewire-lib: cancel flushing isoc context in the laste step to process context callback ALSA: firewire-lib: handle several AMDTP streams in callback handler of IRQ target ALSA: firewire-lib: postpone to start IR context
Applied all patches now.
Although the preempt handling in AMDTP looks a bit suspicious, it should be OK as long as the code has been tested, so I took as is.
I can understand your concern but it works well as long as I tested. In this time I use preemption-disable during processing queued packet in process context but before I had used irq-disable instead.
I depict a call graph to process isoc packet in process context below:
On pcm.ack() or pcm.pointer() callbacks: fw_iso_context_flush_completions() ->struct fw_card_driver.flush_iso_completions() = ohci_flush_iso_completions() ->tasklet_disable() ->context_tasklet() (read registers on 1394 OHCI controller to get info for queued packet) ->flush_iso_completions() ->struct fw_iso_context.callback.sc() = amdtp_master_callback() ->out_stream_callback() (for irq-target) ->fw_iso_context_flush_completions() ->... ->out_stream_callback() or in_stream_callback() (for non irq-target) ->tasklet_enable()
The tasklet of IT context for irq-target AMDTP stream is temporarily disabled when flushing isoc packets queued for the context. The tasklet doesn't run even if it's queued in hw IRQ context. In a point to processing queued packet for several isoc contexts in the same domain, I have no concern without any irq-flag handling for local cpu.
1394 OHCI controller still continue isoc cycle during the above call graph (= 125 usec interval according to 24.576 MHz clock). Actually the number of queued packets for non-irq-target AMDTP stream can be slightly different from the number for irq-target AMDTP stream by one or two packets without any interruption. In a case that any interruption occurs after processing queued packets for the irq-target stream, it's likely to process largely different number of queued packets for the rest of AMDTP streams in the same domain after resumed. It's desirable not to make such count gap between streams in the same domain and I leave it for my future work.
In this time the count gap is allowed. I use kernel preemption to avoid the interruption but to catch hw IRQ from 1394 OHCI controller (and the other hardware).
In another point, there's a race condition against flushing queued packet in runtime between several PCM substreams for AMDTP streams in the same domain. ALSA PCM core executes pcm.pointer and pcm.ack callback under spin lock of runtime of PCM substream, thus the race is controlled for operations to single PCM substream. However some PCM substreams are associated to the same domain via AMDTP streams. At present I never add any solution for this race.
Thanks
Takashi Sakamoto
Hi,
On Sun, Oct 20, 2019 at 05:37:19PM +0900, Takashi Sakamoto wrote:
On Sat, Oct 19, 2019 at 09:22:16AM +0200, Takashi Iwai wrote:
Although the preempt handling in AMDTP looks a bit suspicious, it should be OK as long as the code has been tested, so I took as is.
I can understand your concern but it works well as long as I tested. In this time I use preemption-disable during processing queued packet in process context but before I had used irq-disable instead.
I depict a call graph to process isoc packet in process context below:
On pcm.ack() or pcm.pointer() callbacks: fw_iso_context_flush_completions() ->struct fw_card_driver.flush_iso_completions() = ohci_flush_iso_completions() ->tasklet_disable() ->context_tasklet() (read registers on 1394 OHCI controller to get info for queued packet) ->flush_iso_completions() ->struct fw_iso_context.callback.sc() = amdtp_master_callback() ->out_stream_callback() (for irq-target) ->fw_iso_context_flush_completions() ->... ->out_stream_callback() or in_stream_callback() (for non irq-target) ->tasklet_enable()
The tasklet of IT context for irq-target AMDTP stream is temporarily disabled when flushing isoc packets queued for the context. The tasklet doesn't run even if it's queued in hw IRQ context. In a point to processing queued packet for several isoc contexts in the same domain, I have no concern without any irq-flag handling for local cpu.
1394 OHCI controller still continue isoc cycle during the above call graph (= 125 usec interval according to 24.576 MHz clock). Actually the number of queued packets for non-irq-target AMDTP stream can be slightly different from the number for irq-target AMDTP stream by one or two packets without any interruption. In a case that any interruption occurs after processing queued packets for the irq-target stream, it's likely to process largely different number of queued packets for the rest of AMDTP streams in the same domain after resumed. It's desirable not to make such count gap between streams in the same domain and I leave it for my future work.
In this time the count gap is allowed. I use kernel preemption to avoid the interruption but to catch hw IRQ from 1394 OHCI controller (and the other hardware).
In another point, there's a race condition against flushing queued packet in runtime between several PCM substreams for AMDTP streams in the same domain. ALSA PCM core executes pcm.pointer and pcm.ack callback under spin lock of runtime of PCM substream, thus the race is controlled for operations to single PCM substream. However some PCM substreams are associated to the same domain via AMDTP streams. At present I never add any solution for this race.
I realize that this race is managed as well, by a call of test_and_set_bit_lock(). When operations for several PCM substream call pcm.pointer or pcm.ack simultaneously, one of them can flush queued packets. I refine the above call graph:
On pcm.ack() or pcm.pointer() callbacks: fw_iso_context_flush_completions() ->struct fw_card_driver.flush_iso_completions() = ohci_flush_iso_completions() ->tasklet_disable() if (!test_and_set_bit_lock()) ->context_tasklet() (read registers on 1394 OHCI controller to get info for queued packet) ->flush_iso_completions() ->struct fw_iso_context.callback.sc() = amdtp_master_callback() ->out_stream_callback() (for irq-target) ->fw_iso_context_flush_completions() ->... ->out_stream_callback() or in_stream_callback() (for non irq-target) ->clear_bit_unlock() ->tasklet_enable()
Regards
Takashi Sakamoto
participants (2)
-
Takashi Iwai -
Takashi Sakamoto