Although the value of FDF is used just for outgoing stream, the assignment to union member is done for both directions of stream. At present this causes no issue because the value of same position is reassigned later for opposite stream. However, it's better to add if statement.

Fixes: d3d10a4a1b19 ("ALSA: firewire-lib: use union for directional parameters") Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/firewire/amdtp-am824.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/sound/firewire/amdtp-am824.c b/sound/firewire/amdtp-am824.c index 67d735e9a6a4..fea92e148790 100644 --- a/sound/firewire/amdtp-am824.c +++ b/sound/firewire/amdtp-am824.c @@ -82,7 +82,8 @@ int amdtp_am824_set_parameters(struct amdtp_stream *s, unsigned int rate, if (err < 0) return err;

- s->ctx_data.rx.fdf = AMDTP_FDF_AM824 | s->sfc; + if (s->direction == AMDTP_OUT_STREAM) + s->ctx_data.rx.fdf = AMDTP_FDF_AM824 | s->sfc;

p->pcm_channels = pcm_channels; p->midi_ports = midi_ports;

In current implementation, AMDTP domain structure and AMDTP stream structure has one way of reference from the former to the latter. For future extension, bidirectional reference is needed.

This commit adds a member into stream structure to refer to domain structure to which the stream belongs.

Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/firewire/amdtp-stream.c | 75 ++++++++++------------------------- sound/firewire/amdtp-stream.h | 3 ++ 2 files changed, 23 insertions(+), 55 deletions(-)

diff --git a/sound/firewire/amdtp-stream.c b/sound/firewire/amdtp-stream.c index fcde01b54d11..ce63ff6b7f03 100644 --- a/sound/firewire/amdtp-stream.c +++ b/sound/firewire/amdtp-stream.c @@ -793,14 +793,6 @@ 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) @@ -810,7 +802,6 @@ static void out_stream_callback(struct fw_iso_context *context, u32 tstamp, 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) @@ -823,10 +814,6 @@ 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; @@ -845,7 +832,7 @@ static void out_stream_callback(struct fw_iso_context *context, u32 tstamp, desc->data_blocks, desc->data_block_counter, syt, i);

- if (is_irq_target) { + if (s == s->domain->irq_target) { event_count += desc->data_blocks; if (event_count >= events_per_period) { event_count -= events_per_period; @@ -898,12 +885,12 @@ static void in_stream_callback(struct fw_iso_context *context, u32 tstamp, } }

-static void amdtp_stream_master_callback(struct fw_iso_context *context, - u32 tstamp, size_t header_length, - void *header, void *private_data) +static void irq_target_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 *irq_target = private_data; + struct amdtp_domain *d = irq_target->domain; struct amdtp_stream *s;

out_stream_callback(context, tstamp, header_length, header, irq_target); @@ -952,7 +939,10 @@ static void amdtp_stream_first_callback(struct fw_iso_context *context, } else { cycle = compute_it_cycle(*ctx_header, s->queue_size);

- context->callback.sc = out_stream_callback; + if (s == s->domain->irq_target) + context->callback.sc = irq_target_callback; + else + context->callback.sc = out_stream_callback; }

s->start_cycle = cycle; @@ -960,32 +950,11 @@ 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. * @start_cycle: the isochronous cycle to start the context. Start immediately * if negative value is given. * @@ -994,7 +963,6 @@ static void amdtp_stream_master_first_callback(struct fw_iso_context *context, * 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, int start_cycle) { static const struct { @@ -1009,15 +977,14 @@ static int amdtp_stream_start(struct amdtp_stream *s, int channel, int speed, [CIP_SFC_88200] = { 0, 67 }, [CIP_SFC_176400] = { 0, 67 }, }; - unsigned int events_per_buffer = d->events_per_buffer; - unsigned int events_per_period = d->events_per_period; + bool is_irq_target = (s == s->domain->irq_target); + unsigned int events_per_buffer; + unsigned int 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);

@@ -1068,6 +1035,8 @@ static int amdtp_stream_start(struct amdtp_stream *s, int channel, int speed, // This is a case that AMDTP streams in domain run just for MIDI // substream. Use the number of events equivalent to 10 msec as // interval of hardware IRQ. + events_per_buffer = s->domain->events_per_buffer; + events_per_period = s->domain->events_per_period; if (events_per_period == 0) events_per_period = amdtp_rate_table[s->sfc] / 100; if (events_per_buffer == 0) @@ -1086,16 +1055,11 @@ static int amdtp_stream_start(struct amdtp_stream *s, int channel, int speed, 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, - ctx_cb, ctx_data); + amdtp_stream_first_callback, s); if (IS_ERR(s->context)) { err = PTR_ERR(s->context); if (err == -EBUSY) @@ -1340,6 +1304,7 @@ int amdtp_domain_add_stream(struct amdtp_domain *d, struct amdtp_stream *s,

s->channel = channel; s->speed = speed; + s->domain = d;

return 0; } @@ -1428,15 +1393,15 @@ int amdtp_domain_start(struct amdtp_domain *d, unsigned int ir_delay_cycle) }

if (s != d->irq_target) { - err = amdtp_stream_start(s, s->channel, s->speed, d, - false, cycle_match); + err = amdtp_stream_start(s, s->channel, s->speed, + cycle_match); if (err < 0) goto error; } }

s = d->irq_target; - err = amdtp_stream_start(s, s->channel, s->speed, d, true, -1); + err = amdtp_stream_start(s, s->channel, s->speed, -1); if (err < 0) goto error;

diff --git a/sound/firewire/amdtp-stream.h b/sound/firewire/amdtp-stream.h index f2d44e2dc3c8..477fbfe713e5 100644 --- a/sound/firewire/amdtp-stream.h +++ b/sound/firewire/amdtp-stream.h @@ -108,6 +108,8 @@ typedef unsigned int (*amdtp_stream_process_ctx_payloads_t)( const struct pkt_desc *desc, unsigned int packets, struct snd_pcm_substream *pcm); + +struct amdtp_domain; struct amdtp_stream { struct fw_unit *unit; enum cip_flags flags; @@ -180,6 +182,7 @@ struct amdtp_stream { int channel; int speed; struct list_head list; + struct amdtp_domain *domain; };

int amdtp_stream_init(struct amdtp_stream *s, struct fw_unit *unit,

In current implementation for outgoing AMDTP packet, the value of syt field in CIP header is computed when calculating syt offset. For future extension, it's convenient to split the computation and calculation.

This commit splits them.

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

diff --git a/sound/firewire/amdtp-stream.c b/sound/firewire/amdtp-stream.c index 6130c240ff33..1605ea4301ce 100644 --- a/sound/firewire/amdtp-stream.c +++ b/sound/firewire/amdtp-stream.c @@ -383,10 +383,9 @@ static unsigned int calculate_data_blocks(struct amdtp_stream *s, return data_blocks; }

-static unsigned int calculate_syt(struct amdtp_stream *s, - unsigned int cycle) +static unsigned int calculate_syt_offset(struct amdtp_stream *s) { - unsigned int syt_offset, phase, index, syt; + unsigned int syt_offset, phase, index;

if (s->ctx_data.rx.last_syt_offset < TICKS_PER_CYCLE) { if (!cip_sfc_is_base_44100(s->sfc)) @@ -416,15 +415,10 @@ static unsigned int calculate_syt(struct amdtp_stream *s, syt_offset = s->ctx_data.rx.last_syt_offset - TICKS_PER_CYCLE; s->ctx_data.rx.last_syt_offset = syt_offset;

- if (syt_offset < TICKS_PER_CYCLE) { - syt_offset += s->ctx_data.rx.transfer_delay; - syt = (cycle + syt_offset / TICKS_PER_CYCLE) << 12; - syt += syt_offset % TICKS_PER_CYCLE; + if (syt_offset >= TICKS_PER_CYCLE) + syt_offset = CIP_SYT_NO_INFO;

- return syt & CIP_SYT_MASK; - } else { - return CIP_SYT_NO_INFO; - } + return syt_offset; }

static void update_pcm_pointers(struct amdtp_stream *s, @@ -740,6 +734,17 @@ static int generate_device_pkt_descs(struct amdtp_stream *s, return 0; }

+static unsigned int compute_syt(unsigned int syt_offset, unsigned int cycle, + unsigned int transfer_delay) +{ + unsigned int syt; + + syt_offset += transfer_delay; + syt = ((cycle + syt_offset / TICKS_PER_CYCLE) << 12) | + (syt_offset % TICKS_PER_CYCLE); + return syt & CIP_SYT_MASK; +} + static void generate_ideal_pkt_descs(struct amdtp_stream *s, struct pkt_desc *descs, const __be32 *ctx_header, @@ -751,9 +756,16 @@ static void generate_ideal_pkt_descs(struct amdtp_stream *s, for (i = 0; i < packets; ++i) { struct pkt_desc *desc = descs + i; unsigned int index = (s->packet_index + i) % s->queue_size; + unsigned int syt_offset;

desc->cycle = compute_it_cycle(*ctx_header, s->queue_size); - desc->syt = calculate_syt(s, desc->cycle); + syt_offset = calculate_syt_offset(s); + if (syt_offset != CIP_SYT_NO_INFO) { + desc->syt = compute_syt(syt_offset, desc->cycle, + s->ctx_data.rx.transfer_delay); + } else { + desc->syt = syt_offset; + } desc->data_blocks = calculate_data_blocks(s, desc->syt);

if (s->flags & CIP_DBC_IS_END_EVENT)

When calculating the number of data blocks per packet, some states are stored in AMDTP stream structure. This is inconvenient when reuse the calculation from non-stream structure.

This commit applies refactoring to helper function for the calculation so that the function doesn't touch AMDTP stream structure.

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

diff --git a/sound/firewire/amdtp-stream.c b/sound/firewire/amdtp-stream.c index 9041510cb6aa..efd1f2a40cf1 100644 --- a/sound/firewire/amdtp-stream.c +++ b/sound/firewire/amdtp-stream.c @@ -339,25 +339,26 @@ void amdtp_stream_pcm_prepare(struct amdtp_stream *s) } EXPORT_SYMBOL(amdtp_stream_pcm_prepare);

-static unsigned int calculate_data_blocks(struct amdtp_stream *s, - unsigned int syt) +static unsigned int calculate_data_blocks(unsigned int *data_block_state, + bool is_blocking, bool is_no_info, + unsigned int syt_interval, enum cip_sfc sfc) { - unsigned int phase, data_blocks; + unsigned int data_blocks;

/* Blocking mode. */ - if (s->flags & CIP_BLOCKING) { + if (is_blocking) { /* This module generate empty packet for 'no data'. */ - if (syt == CIP_SYT_NO_INFO) + if (is_no_info) data_blocks = 0; else - data_blocks = s->syt_interval; + data_blocks = syt_interval; /* Non-blocking mode. */ } else { - if (!cip_sfc_is_base_44100(s->sfc)) { + if (!cip_sfc_is_base_44100(sfc)) { // Sample_rate / 8000 is an integer, and precomputed. - data_blocks = s->ctx_data.rx.data_block_state; + data_blocks = *data_block_state; } else { - phase = s->ctx_data.rx.data_block_state; + unsigned int phase = *data_block_state;

/* * This calculates the number of data blocks per packet so that @@ -367,16 +368,16 @@ static unsigned int calculate_data_blocks(struct amdtp_stream *s, * as possible in the sequence (to prevent underruns of the * device's buffer). */ - if (s->sfc == CIP_SFC_44100) + if (sfc == CIP_SFC_44100) /* 6 6 5 6 5 6 5 ... */ data_blocks = 5 + ((phase & 1) ^ (phase == 0 || phase >= 40)); else /* 12 11 11 11 11 ... or 23 22 22 22 22 ... */ - data_blocks = 11 * (s->sfc >> 1) + (phase == 0); - if (++phase >= (80 >> (s->sfc >> 1))) + data_blocks = 11 * (sfc >> 1) + (phase == 0); + if (++phase >= (80 >> (sfc >> 1))) phase = 0; - s->ctx_data.rx.data_block_state = phase; + *data_block_state = phase; } }

@@ -769,7 +770,11 @@ static void generate_ideal_pkt_descs(struct amdtp_stream *s, } else { desc->syt = syt_offset; } - desc->data_blocks = calculate_data_blocks(s, desc->syt); + desc->data_blocks = + calculate_data_blocks(&s->ctx_data.rx.data_block_state, + !!(s->flags & CIP_BLOCKING), + desc->syt == CIP_SYT_NO_INFO, + s->syt_interval, s->sfc);

if (s->flags & CIP_DBC_IS_END_EVENT) dbc = (dbc + desc->data_blocks) & 0xff;

In current implementation, sequence of syt offset and the number of data blocks is generated when packets for outgoing stream are going to be queued.

This commit generates and pools the sequence independently of the processing of outgoing packets for future extension.

Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/firewire/amdtp-stream.c | 68 +++++++++++++++++++++++++++++++++++ sound/firewire/amdtp-stream.h | 6 ++++ 2 files changed, 74 insertions(+)

diff --git a/sound/firewire/amdtp-stream.c b/sound/firewire/amdtp-stream.c index f1c8611cfc70..a2af598e9b9a 100644 --- a/sound/firewire/amdtp-stream.c +++ b/sound/firewire/amdtp-stream.c @@ -905,14 +905,63 @@ static void in_stream_callback(struct fw_iso_context *context, u32 tstamp, } }

+static void pool_ideal_seq_descs(struct amdtp_domain *d, unsigned int packets) +{ + struct amdtp_stream *irq_target = d->irq_target; + unsigned int seq_tail = d->seq_tail; + unsigned int seq_size = d->seq_size; + unsigned int min_avail; + struct amdtp_stream *s; + + min_avail = d->seq_size; + list_for_each_entry(s, &d->streams, list) { + unsigned int seq_index; + unsigned int avail; + + if (s->direction == AMDTP_IN_STREAM) + continue; + + seq_index = s->ctx_data.rx.seq_index; + avail = d->seq_tail; + if (seq_index > avail) + avail += d->seq_size; + avail -= seq_index; + + if (avail < min_avail) + min_avail = avail; + } + + while (min_avail < packets) { + struct seq_desc *desc = d->seq_descs + seq_tail; + + desc->syt_offset = calculate_syt_offset(&d->last_syt_offset, + &d->syt_offset_state, irq_target->sfc); + desc->data_blocks = calculate_data_blocks(&d->data_block_state, + !!(irq_target->flags & CIP_BLOCKING), + desc->syt_offset == CIP_SYT_NO_INFO, + irq_target->syt_interval, irq_target->sfc); + + ++seq_tail; + seq_tail %= seq_size; + + ++min_avail; + } + + d->seq_tail = seq_tail; +} + static void irq_target_callback(struct fw_iso_context *context, u32 tstamp, size_t header_length, void *header, void *private_data) { struct amdtp_stream *irq_target = private_data; struct amdtp_domain *d = irq_target->domain; + unsigned int packets = header_length / sizeof(__be32); struct amdtp_stream *s;

+ // Record enough entries with extra 3 cycles at least. + pool_ideal_seq_descs(d, packets + 3); + out_stream_callback(context, tstamp, header_length, header, irq_target); if (amdtp_streaming_error(irq_target)) goto error; @@ -1344,6 +1393,18 @@ static int get_current_cycle_time(struct fw_card *fw_card, int *cur_cycle) */ int amdtp_domain_start(struct amdtp_domain *d, unsigned int ir_delay_cycle) { + static const struct { + unsigned int data_block; + unsigned int syt_offset; + } *entry, initial_state[] = { + [CIP_SFC_32000] = { 4, 3072 }, + [CIP_SFC_48000] = { 6, 1024 }, + [CIP_SFC_96000] = { 12, 1024 }, + [CIP_SFC_192000] = { 24, 1024 }, + [CIP_SFC_44100] = { 0, 67 }, + [CIP_SFC_88200] = { 0, 67 }, + [CIP_SFC_176400] = { 0, 67 }, + }; unsigned int events_per_buffer = d->events_per_buffer; unsigned int events_per_period = d->events_per_period; unsigned int idle_irq_interval; @@ -1378,6 +1439,11 @@ int amdtp_domain_start(struct amdtp_domain *d, unsigned int ir_delay_cycle) d->seq_size = queue_size; d->seq_tail = 0;

+ entry = &initial_state[s->sfc]; + d->data_block_state = entry->data_block; + d->syt_offset_state = entry->syt_offset; + d->last_syt_offset = TICKS_PER_CYCLE; + if (ir_delay_cycle > 0) { struct fw_card *fw_card = fw_parent_device(s->unit)->card;

@@ -1414,6 +1480,7 @@ int amdtp_domain_start(struct amdtp_domain *d, unsigned int ir_delay_cycle) } else { // IT context starts immediately. cycle_match = -1; + s->ctx_data.rx.seq_index = 0; }

if (s != d->irq_target) { @@ -1427,6 +1494,7 @@ int amdtp_domain_start(struct amdtp_domain *d, unsigned int ir_delay_cycle) s = d->irq_target; s->ctx_data.rx.events_per_period = events_per_period; s->ctx_data.rx.event_count = 0; + s->ctx_data.rx.seq_index = 0;

idle_irq_interval = DIV_ROUND_UP(CYCLES_PER_SECOND * events_per_period, amdtp_rate_table[d->irq_target->sfc]); diff --git a/sound/firewire/amdtp-stream.h b/sound/firewire/amdtp-stream.h index 84a01efa5a85..11cff4cafd90 100644 --- a/sound/firewire/amdtp-stream.h +++ b/sound/firewire/amdtp-stream.h @@ -138,6 +138,8 @@ struct amdtp_stream { struct { // To calculate CIP data blocks and tstamp. unsigned int transfer_delay; + unsigned int seq_index; + unsigned int data_block_state; unsigned int last_syt_offset; unsigned int syt_offset_state; @@ -292,6 +294,10 @@ struct amdtp_domain { struct seq_desc *seq_descs; unsigned int seq_size; unsigned int seq_tail; + + unsigned int data_block_state; + unsigned int syt_offset_state; + unsigned int last_syt_offset; };

int amdtp_domain_init(struct amdtp_domain *d);