[alsa-devel] [PATCH 0/6] ALSA: firewire-lib: unify handlers for incoming packet
Hi,
In IR context of Linux FireWire subsystem, some quadlets of packet payload can be handled as a part of context header. As a result context payload can just include the rest of packet payload.
This patchset uses the mechanism to unify handlers of incoming packet for with-CIP and without-CIP headers.
Takashi Sakamoto (6): ALSA: firewire-lib: use clear name for variable of CIP header ALSA: firewire-lib: calculate the length of packet payload in packet handler ALSA: firewire-lib: compute pointer to payload buffer in context handler ALSA: firewire-lib: split helper function to check incoming CIP header ALSA: firewire-lib: use 16 bytes IR context header to separate CIP header ALSA: firewire-lib: unify packet handler for IR context
sound/firewire/amdtp-stream.c | 180 ++++++++++++++++++---------------- sound/firewire/amdtp-stream.h | 8 +- 2 files changed, 97 insertions(+), 91 deletions(-)
This commit is to distinguish variable of CIP header from variable of isochronous context header.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/firewire/amdtp-stream.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/sound/firewire/amdtp-stream.c b/sound/firewire/amdtp-stream.c index f43943fd962d..020edf2b1726 100644 --- a/sound/firewire/amdtp-stream.c +++ b/sound/firewire/amdtp-stream.c @@ -286,15 +286,15 @@ EXPORT_SYMBOL(amdtp_stream_set_parameters); unsigned int amdtp_stream_get_max_payload(struct amdtp_stream *s) { unsigned int multiplier = 1; - unsigned int header_size = 0; + unsigned int cip_header_size = 0;
if (s->flags & CIP_JUMBO_PAYLOAD) multiplier = 5; if (!(s->flags & CIP_NO_HEADER)) - header_size = 8; + cip_header_size = sizeof(__be32) * 2;
- return header_size + - s->syt_interval * s->data_block_quadlets * 4 * multiplier; + return cip_header_size + + s->syt_interval * s->data_block_quadlets * sizeof(__be32) * multiplier; } EXPORT_SYMBOL(amdtp_stream_get_max_payload);
In current packet handler, the length of payload is given as an argument of callback function, however this value is just required to process payload of transferred isoc packet, thus just for IR context.
This commit replaces the argument for payload of packet with the argument of context header. As a result, the length of payload is computed in packet handler for IR context.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/firewire/amdtp-stream.c | 49 ++++++++++++++++------------------- sound/firewire/amdtp-stream.h | 5 ++-- 2 files changed, 25 insertions(+), 29 deletions(-)
diff --git a/sound/firewire/amdtp-stream.c b/sound/firewire/amdtp-stream.c index 020edf2b1726..4584525a7f30 100644 --- a/sound/firewire/amdtp-stream.c +++ b/sound/firewire/amdtp-stream.c @@ -474,14 +474,14 @@ static inline int queue_in_packet(struct amdtp_stream *s) return queue_packet(s, s->ctx_data.tx.max_payload_length); }
-static int handle_out_packet(struct amdtp_stream *s, - unsigned int payload_length, unsigned int cycle, - unsigned int index) +static int handle_out_packet(struct amdtp_stream *s, unsigned int cycle, + const __be32 *ctx_header, unsigned int index) { __be32 *buffer; unsigned int syt; unsigned int data_blocks; unsigned int pcm_frames; + unsigned int payload_length; struct snd_pcm_substream *pcm;
buffer = s->buffer.packets[s->packet_index].buffer; @@ -521,13 +521,14 @@ static int handle_out_packet(struct amdtp_stream *s, }
static int handle_out_packet_without_header(struct amdtp_stream *s, - unsigned int payload_length, unsigned int cycle, - unsigned int index) + unsigned int cycle, const __be32 *ctx_header, + unsigned int index) { __be32 *buffer; unsigned int syt; unsigned int data_blocks; unsigned int pcm_frames; + unsigned int payload_length; struct snd_pcm_substream *pcm;
buffer = s->buffer.packets[s->packet_index].buffer; @@ -551,11 +552,11 @@ static int handle_out_packet_without_header(struct amdtp_stream *s, return 0; }
-static int handle_in_packet(struct amdtp_stream *s, - unsigned int payload_length, unsigned int cycle, - unsigned int index) +static int handle_in_packet(struct amdtp_stream *s, unsigned int cycle, + const __be32 *ctx_header, unsigned int index) { __be32 *buffer; + unsigned int payload_length; u32 cip_header[2]; unsigned int sph, fmt, fdf, syt; unsigned int data_block_quadlets, data_block_counter, dbc_interval; @@ -564,6 +565,14 @@ static int handle_in_packet(struct amdtp_stream *s, unsigned int pcm_frames; bool lost;
+ payload_length = be32_to_cpu(ctx_header[0]) >> ISO_DATA_LENGTH_SHIFT; + if (payload_length > s->ctx_data.tx.max_payload_length) { + dev_err(&s->unit->device, + "Detect jumbo payload: %04x %04x\n", + payload_length, s->ctx_data.tx.max_payload_length); + return -EIO; + } + buffer = s->buffer.packets[s->packet_index].buffer; cip_header[0] = be32_to_cpu(buffer[0]); cip_header[1] = be32_to_cpu(buffer[1]); @@ -668,14 +677,16 @@ static int handle_in_packet(struct amdtp_stream *s, }
static int handle_in_packet_without_header(struct amdtp_stream *s, - unsigned int payload_length, unsigned int cycle, - unsigned int index) + unsigned int cycle, const __be32 *ctx_header, + unsigned int index) { __be32 *buffer; + unsigned int payload_length; unsigned int data_blocks; struct snd_pcm_substream *pcm; unsigned int pcm_frames;
+ payload_length = be32_to_cpu(ctx_header[0]) >> ISO_DATA_LENGTH_SHIFT; buffer = s->buffer.packets[s->packet_index].buffer; data_blocks = payload_length / sizeof(__be32) / s->data_block_quadlets;
@@ -745,7 +756,7 @@ static void out_stream_callback(struct fw_iso_context *context, u32 tstamp,
cycle = compute_it_cycle(*ctx_header);
- if (s->handle_packet(s, 0, cycle, i) < 0) { + if (s->handle_packet(s, cycle, ctx_header, i) < 0) { cancel_stream(s); return; } @@ -762,7 +773,6 @@ static void in_stream_callback(struct fw_iso_context *context, u32 tstamp, { struct amdtp_stream *s = private_data; unsigned int i, packets; - unsigned int payload_length, max_payload_length; __be32 *ctx_header = header;
if (s->packet_index < 0) @@ -771,25 +781,12 @@ static void in_stream_callback(struct fw_iso_context *context, u32 tstamp, // The number of packets in buffer. packets = header_length / s->ctx_data.tx.ctx_header_size;
- /* For buffer-over-run prevention. */ - max_payload_length = s->ctx_data.tx.max_payload_length; - for (i = 0; i < packets; i++) { - u32 iso_header = be32_to_cpu(ctx_header[0]); u32 cycle;
cycle = compute_cycle_count(ctx_header[1]);
- /* The number of bytes in this packet */ - payload_length = iso_header >> ISO_DATA_LENGTH_SHIFT; - if (payload_length > max_payload_length) { - dev_err(&s->unit->device, - "Detect jumbo payload: %04x %04x\n", - payload_length, max_payload_length); - break; - } - - if (s->handle_packet(s, payload_length, cycle, i) < 0) + if (s->handle_packet(s, cycle, ctx_header, i) < 0) break;
ctx_header += s->ctx_data.tx.ctx_header_size / sizeof(*ctx_header); diff --git a/sound/firewire/amdtp-stream.h b/sound/firewire/amdtp-stream.h index 1945ef59ab92..d317fdc6ab5f 100644 --- a/sound/firewire/amdtp-stream.h +++ b/sound/firewire/amdtp-stream.h @@ -108,9 +108,8 @@ struct amdtp_stream { struct iso_packets_buffer buffer; int packet_index; int tag; - int (*handle_packet)(struct amdtp_stream *s, - unsigned int payload_quadlets, unsigned int cycle, - unsigned int index); + int (*handle_packet)(struct amdtp_stream *s, unsigned int cycle, + const __be32 *ctx_header, unsigned int index); union { struct { unsigned int ctx_header_size;
The value of pointer to payload buffer is computed in each packet handler, however the pointer can be decided before call of packet handler.
This commit adds an argument for the pointer to the packet handler to reduce codes to compute for the pointer.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/firewire/amdtp-stream.c | 28 +++++++++++++--------------- sound/firewire/amdtp-stream.h | 3 ++- 2 files changed, 15 insertions(+), 16 deletions(-)
diff --git a/sound/firewire/amdtp-stream.c b/sound/firewire/amdtp-stream.c index 4584525a7f30..ab9dc7e9ffa4 100644 --- a/sound/firewire/amdtp-stream.c +++ b/sound/firewire/amdtp-stream.c @@ -475,16 +475,15 @@ static inline int queue_in_packet(struct amdtp_stream *s) }
static int handle_out_packet(struct amdtp_stream *s, unsigned int cycle, - const __be32 *ctx_header, unsigned int index) + const __be32 *ctx_header, __be32 *buffer, + unsigned int index) { - __be32 *buffer; unsigned int syt; unsigned int data_blocks; unsigned int pcm_frames; unsigned int payload_length; 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); @@ -522,16 +521,14 @@ static int handle_out_packet(struct amdtp_stream *s, unsigned int cycle,
static int handle_out_packet_without_header(struct amdtp_stream *s, unsigned int cycle, const __be32 *ctx_header, - unsigned int index) + __be32 *buffer, unsigned int index) { - __be32 *buffer; unsigned int syt; unsigned int data_blocks; unsigned int pcm_frames; unsigned int payload_length; 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, data_blocks, &syt); @@ -553,9 +550,9 @@ static int handle_out_packet_without_header(struct amdtp_stream *s, }
static int handle_in_packet(struct amdtp_stream *s, unsigned int cycle, - const __be32 *ctx_header, unsigned int index) + const __be32 *ctx_header, __be32 *buffer, + unsigned int index) { - __be32 *buffer; unsigned int payload_length; u32 cip_header[2]; unsigned int sph, fmt, fdf, syt; @@ -573,7 +570,6 @@ static int handle_in_packet(struct amdtp_stream *s, unsigned int cycle, return -EIO; }
- buffer = s->buffer.packets[s->packet_index].buffer; cip_header[0] = be32_to_cpu(buffer[0]); cip_header[1] = be32_to_cpu(buffer[1]);
@@ -678,17 +674,15 @@ static int handle_in_packet(struct amdtp_stream *s, unsigned int cycle,
static int handle_in_packet_without_header(struct amdtp_stream *s, unsigned int cycle, const __be32 *ctx_header, - unsigned int index) + __be32 *buffer, unsigned int index) { - __be32 *buffer; unsigned int payload_length; unsigned int data_blocks; struct snd_pcm_substream *pcm; unsigned int pcm_frames;
payload_length = be32_to_cpu(ctx_header[0]) >> ISO_DATA_LENGTH_SHIFT; - buffer = s->buffer.packets[s->packet_index].buffer; - data_blocks = payload_length / sizeof(__be32) / s->data_block_quadlets; + data_blocks = payload_length / 4 / s->data_block_quadlets;
trace_amdtp_packet(s, cycle, NULL, payload_length, data_blocks, index);
@@ -753,10 +747,12 @@ static void out_stream_callback(struct fw_iso_context *context, u32 tstamp,
for (i = 0; i < packets; ++i) { u32 cycle; + __be32 *buffer;
cycle = compute_it_cycle(*ctx_header); + buffer = s->buffer.packets[s->packet_index].buffer;
- if (s->handle_packet(s, cycle, ctx_header, i) < 0) { + if (s->handle_packet(s, cycle, ctx_header, buffer, i) < 0) { cancel_stream(s); return; } @@ -783,10 +779,12 @@ static void in_stream_callback(struct fw_iso_context *context, u32 tstamp,
for (i = 0; i < packets; i++) { u32 cycle; + __be32 *buffer;
cycle = compute_cycle_count(ctx_header[1]); + buffer = s->buffer.packets[s->packet_index].buffer;
- if (s->handle_packet(s, cycle, ctx_header, i) < 0) + if (s->handle_packet(s, cycle, ctx_header, buffer, i) < 0) break;
ctx_header += s->ctx_data.tx.ctx_header_size / sizeof(*ctx_header); diff --git a/sound/firewire/amdtp-stream.h b/sound/firewire/amdtp-stream.h index d317fdc6ab5f..5aa9683593d2 100644 --- a/sound/firewire/amdtp-stream.h +++ b/sound/firewire/amdtp-stream.h @@ -109,7 +109,8 @@ struct amdtp_stream { int packet_index; int tag; int (*handle_packet)(struct amdtp_stream *s, unsigned int cycle, - const __be32 *ctx_header, unsigned int index); + const __be32 *ctx_header, __be32 *buffer, + unsigned int index); union { struct { unsigned int ctx_header_size;
A parser for CIP header in incoming packet is enough large.
This commit splits it into a helper function to better looks of packet handler.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/firewire/amdtp-stream.c | 100 ++++++++++++++++++++-------------- 1 file changed, 60 insertions(+), 40 deletions(-)
diff --git a/sound/firewire/amdtp-stream.c b/sound/firewire/amdtp-stream.c index ab9dc7e9ffa4..e9976a877944 100644 --- a/sound/firewire/amdtp-stream.c +++ b/sound/firewire/amdtp-stream.c @@ -549,29 +549,19 @@ static int handle_out_packet_without_header(struct amdtp_stream *s, return 0; }
-static int handle_in_packet(struct amdtp_stream *s, unsigned int cycle, - const __be32 *ctx_header, __be32 *buffer, - unsigned int index) +static int check_cip_header(struct amdtp_stream *s, const __be32 *buf, + unsigned int payload_length, + unsigned int *data_blocks, unsigned int *syt) { - unsigned int payload_length; u32 cip_header[2]; - unsigned int sph, 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; + unsigned int sph; + unsigned int fmt; + unsigned int fdf; + unsigned int data_block_counter; bool lost;
- payload_length = be32_to_cpu(ctx_header[0]) >> ISO_DATA_LENGTH_SHIFT; - if (payload_length > s->ctx_data.tx.max_payload_length) { - dev_err(&s->unit->device, - "Detect jumbo payload: %04x %04x\n", - payload_length, s->ctx_data.tx.max_payload_length); - return -EIO; - } - - cip_header[0] = be32_to_cpu(buffer[0]); - cip_header[1] = be32_to_cpu(buffer[1]); + cip_header[0] = be32_to_cpu(buf[0]); + cip_header[1] = be32_to_cpu(buf[1]);
/* * This module supports 'Two-quadlet CIP header with SYT field'. @@ -583,9 +573,7 @@ static int handle_in_packet(struct amdtp_stream *s, unsigned int cycle, dev_info_ratelimited(&s->unit->device, "Invalid CIP header for AMDTP: %08X:%08X\n", cip_header[0], cip_header[1]); - data_blocks = 0; - pcm_frames = 0; - goto end; + return -EAGAIN; }
/* Check valid protocol or not. */ @@ -595,19 +583,17 @@ static int handle_in_packet(struct amdtp_stream *s, unsigned int cycle, dev_info_ratelimited(&s->unit->device, "Detect unexpected protocol: %08x %08x\n", cip_header[0], cip_header[1]); - data_blocks = 0; - pcm_frames = 0; - goto end; + return -EAGAIN; }
/* Calculate data blocks */ fdf = (cip_header[1] & CIP_FDF_MASK) >> CIP_FDF_SHIFT; - if (payload_length < 12 || + if (payload_length < sizeof(__be32) * 2 || (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; + unsigned int data_block_quadlets = + (cip_header[0] & CIP_DBS_MASK) >> CIP_DBS_SHIFT; /* avoid division by zero */ if (data_block_quadlets == 0) { dev_err(&s->unit->device, @@ -618,13 +604,13 @@ static int handle_in_packet(struct amdtp_stream *s, unsigned int cycle, if (s->flags & CIP_WRONG_DBS) data_block_quadlets = s->data_block_quadlets;
- data_blocks = (payload_length / 4 - 2) / + *data_blocks = (payload_length / sizeof(__be32) - 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;
@@ -635,10 +621,12 @@ static int handle_in_packet(struct amdtp_stream *s, unsigned int cycle, } else if (!(s->flags & CIP_DBC_IS_END_EVENT)) { lost = data_block_counter != s->data_block_counter; } else { - if (data_blocks > 0 && s->ctx_data.tx.dbc_interval > 0) + unsigned int dbc_interval; + + if (*data_blocks > 0 && s->ctx_data.tx.dbc_interval > 0) dbc_interval = s->ctx_data.tx.dbc_interval; else - dbc_interval = data_blocks; + dbc_interval = *data_blocks;
lost = data_block_counter != ((s->data_block_counter + dbc_interval) & 0xff); @@ -651,16 +639,48 @@ static int handle_in_packet(struct amdtp_stream *s, unsigned int cycle, return -EIO; }
- trace_amdtp_packet(s, cycle, buffer, payload_length, data_blocks, index); - - syt = be32_to_cpu(buffer[1]) & CIP_SYT_MASK; - pcm_frames = s->process_data_blocks(s, buffer + 2, data_blocks, &syt); + *syt = cip_header[1] & CIP_SYT_MASK;
- if (s->flags & CIP_DBC_IS_END_EVENT) + if (s->flags & CIP_DBC_IS_END_EVENT) { s->data_block_counter = data_block_counter; - else + } else { s->data_block_counter = - (data_block_counter + data_blocks) & 0xff; + (data_block_counter + *data_blocks) & 0xff; + } + + return 0; +} + +static int handle_in_packet(struct amdtp_stream *s, unsigned int cycle, + const __be32 *ctx_header, __be32 *buffer, + unsigned int index) +{ + unsigned int payload_length; + unsigned int syt; + unsigned int data_blocks; + struct snd_pcm_substream *pcm; + unsigned int pcm_frames; + int err; + + payload_length = be32_to_cpu(ctx_header[0]) >> ISO_DATA_LENGTH_SHIFT; + if (payload_length > s->ctx_data.tx.max_payload_length) { + dev_err(&s->unit->device, + "Detect jumbo payload: %04x %04x\n", + payload_length, s->ctx_data.tx.max_payload_length); + return -EIO; + } + + err = check_cip_header(s, buffer, payload_length, &data_blocks, &syt); + if (err < 0) { + if (err != -EAGAIN) + return err; + pcm_frames = 0; + goto end; + } + + trace_amdtp_packet(s, cycle, buffer, payload_length, data_blocks, index); + + pcm_frames = s->process_data_blocks(s, buffer + 2, data_blocks, &syt); end: if (queue_in_packet(s) < 0) return -EIO;
In IR context, some quadlets of packet payload can be included into context header. This is good for packet with CIP header because the context payload buffer can includes data blocks only for with-CIP and without-CIP pakets.
This commit uses 16 bytes IR context header for this purpose.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/firewire/amdtp-stream.c | 37 ++++++++++++++++++++++++----------- sound/firewire/amdtp-stream.h | 2 +- 2 files changed, 27 insertions(+), 12 deletions(-)
diff --git a/sound/firewire/amdtp-stream.c b/sound/firewire/amdtp-stream.c index e9976a877944..fa99210f5a48 100644 --- a/sound/firewire/amdtp-stream.c +++ b/sound/firewire/amdtp-stream.c @@ -56,7 +56,10 @@ #define INTERRUPT_INTERVAL 16 #define QUEUE_LENGTH 48
-#define IR_HEADER_SIZE 8 // For header and timestamp. +// For iso header, tstamp and 2 CIP header. +#define IR_CTX_HEADER_SIZE_CIP 16 +// For iso header and tstamp. +#define IR_CTX_HEADER_SIZE_NO_CIP 8 #define HEADER_TSTAMP_MASK 0x0000ffff
static void pcm_period_tasklet(unsigned long data); @@ -471,7 +474,7 @@ static inline int queue_out_packet(struct amdtp_stream *s,
static inline int queue_in_packet(struct amdtp_stream *s) { - return queue_packet(s, s->ctx_data.tx.max_payload_length); + return queue_packet(s, s->ctx_data.tx.max_ctx_payload_length); }
static int handle_out_packet(struct amdtp_stream *s, unsigned int cycle, @@ -656,6 +659,7 @@ static int handle_in_packet(struct amdtp_stream *s, unsigned int cycle, unsigned int index) { unsigned int payload_length; + const __be32 *cip_header; unsigned int syt; unsigned int data_blocks; struct snd_pcm_substream *pcm; @@ -663,14 +667,17 @@ static int handle_in_packet(struct amdtp_stream *s, unsigned int cycle, int err;
payload_length = be32_to_cpu(ctx_header[0]) >> ISO_DATA_LENGTH_SHIFT; - if (payload_length > s->ctx_data.tx.max_payload_length) { + if (payload_length > s->ctx_data.tx.ctx_header_size + + s->ctx_data.tx.max_ctx_payload_length) { dev_err(&s->unit->device, "Detect jumbo payload: %04x %04x\n", - payload_length, s->ctx_data.tx.max_payload_length); + payload_length, s->ctx_data.tx.max_ctx_payload_length); return -EIO; }
- err = check_cip_header(s, buffer, payload_length, &data_blocks, &syt); + cip_header = ctx_header + 2; + err = check_cip_header(s, cip_header, payload_length, &data_blocks, + &syt); if (err < 0) { if (err != -EAGAIN) return err; @@ -678,9 +685,10 @@ static int handle_in_packet(struct amdtp_stream *s, unsigned int cycle, goto end; }
- trace_amdtp_packet(s, cycle, buffer, payload_length, data_blocks, index); + trace_amdtp_packet(s, cycle, cip_header, payload_length, data_blocks, + index);
- pcm_frames = s->process_data_blocks(s, buffer + 2, data_blocks, &syt); + pcm_frames = s->process_data_blocks(s, buffer, data_blocks, &syt); end: if (queue_in_packet(s) < 0) return -EIO; @@ -883,6 +891,7 @@ int amdtp_stream_start(struct amdtp_stream *s, int channel, int speed) [CIP_SFC_176400] = { 0, 67 }, }; unsigned int ctx_header_size; + unsigned int max_ctx_payload_size; enum dma_data_direction dir; int type, tag, err;
@@ -909,14 +918,21 @@ int amdtp_stream_start(struct amdtp_stream *s, int channel, int speed) if (s->direction == AMDTP_IN_STREAM) { dir = DMA_FROM_DEVICE; type = FW_ISO_CONTEXT_RECEIVE; - ctx_header_size = IR_HEADER_SIZE; + if (!(s->flags & CIP_NO_HEADER)) + ctx_header_size = IR_CTX_HEADER_SIZE_CIP; + else + ctx_header_size = IR_CTX_HEADER_SIZE_NO_CIP; } else { dir = DMA_TO_DEVICE; type = FW_ISO_CONTEXT_TRANSMIT; ctx_header_size = 0; // No effect for IT context. } + + max_ctx_payload_size = amdtp_stream_get_max_payload(s) - + ctx_header_size; + err = iso_packets_buffer_init(&s->buffer, s->unit, QUEUE_LENGTH, - amdtp_stream_get_max_payload(s), dir); + max_ctx_payload_size, dir); if (err < 0) goto err_unlock;
@@ -934,8 +950,7 @@ int amdtp_stream_start(struct amdtp_stream *s, int channel, int speed) amdtp_stream_update(s);
if (s->direction == AMDTP_IN_STREAM) { - s->ctx_data.tx.max_payload_length = - amdtp_stream_get_max_payload(s); + s->ctx_data.tx.max_ctx_payload_length = max_ctx_payload_size; s->ctx_data.tx.ctx_header_size = ctx_header_size; }
diff --git a/sound/firewire/amdtp-stream.h b/sound/firewire/amdtp-stream.h index 5aa9683593d2..234483a31df5 100644 --- a/sound/firewire/amdtp-stream.h +++ b/sound/firewire/amdtp-stream.h @@ -116,7 +116,7 @@ struct amdtp_stream { unsigned int ctx_header_size;
// limit for payload of iso packet. - unsigned int max_payload_length; + unsigned int max_ctx_payload_length;
// For quirks of CIP headers. // Fixed interval of dbc between previos/current
Usage of 16 bytes IR context header allows to handle context payload by the same code for with-CIP and without-CIP packets.
This commit unifies both handlers of with-CIP and without-CIP packets.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/firewire/amdtp-stream.c | 54 ++++++++++------------------------- 1 file changed, 15 insertions(+), 39 deletions(-)
diff --git a/sound/firewire/amdtp-stream.c b/sound/firewire/amdtp-stream.c index fa99210f5a48..2d9c764061d1 100644 --- a/sound/firewire/amdtp-stream.c +++ b/sound/firewire/amdtp-stream.c @@ -676,13 +676,20 @@ static int handle_in_packet(struct amdtp_stream *s, unsigned int cycle, }
cip_header = ctx_header + 2; - err = check_cip_header(s, cip_header, payload_length, &data_blocks, - &syt); - if (err < 0) { - if (err != -EAGAIN) - return err; - pcm_frames = 0; - goto end; + if (!(s->flags & CIP_NO_HEADER)) { + cip_header = &ctx_header[2]; + err = check_cip_header(s, cip_header, payload_length, + &data_blocks, &syt); + if (err < 0) { + if (err != -EAGAIN) + return err; + pcm_frames = 0; + goto end; + } + } else { + cip_header = NULL; + data_blocks = payload_length / 4 / s->data_block_quadlets; + syt = 0; }
trace_amdtp_packet(s, cycle, cip_header, payload_length, data_blocks, @@ -700,33 +707,6 @@ static int handle_in_packet(struct amdtp_stream *s, unsigned int cycle, return 0; }
-static int handle_in_packet_without_header(struct amdtp_stream *s, - unsigned int cycle, const __be32 *ctx_header, - __be32 *buffer, unsigned int index) -{ - unsigned int payload_length; - unsigned int data_blocks; - struct snd_pcm_substream *pcm; - unsigned int pcm_frames; - - payload_length = be32_to_cpu(ctx_header[0]) >> ISO_DATA_LENGTH_SHIFT; - data_blocks = payload_length / 4 / s->data_block_quadlets; - - trace_amdtp_packet(s, cycle, NULL, payload_length, data_blocks, index); - - pcm_frames = s->process_data_blocks(s, buffer, data_blocks, NULL); - s->data_block_counter = (s->data_block_counter + data_blocks) & 0xff; - - if (queue_in_packet(s) < 0) - return -EIO; - - pcm = READ_ONCE(s->pcm); - if (pcm && pcm_frames > 0) - update_pcm_pointers(s, pcm, pcm_frames); - - return 0; -} - // In CYCLE_TIMER register of IEEE 1394, 7 bits are used to represent second. On // the other hand, in DMA descriptors of 1394 OHCI, 3 bits are used to represent // it. Thus, via Linux firewire subsystem, we can get the 3 bits for second. @@ -812,7 +792,7 @@ static void in_stream_callback(struct fw_iso_context *context, u32 tstamp, cycle = compute_cycle_count(ctx_header[1]); buffer = s->buffer.packets[s->packet_index].buffer;
- if (s->handle_packet(s, cycle, ctx_header, buffer, i) < 0) + if (handle_in_packet(s, cycle, ctx_header, buffer, i) < 0) break;
ctx_header += s->ctx_data.tx.ctx_header_size / sizeof(*ctx_header); @@ -847,10 +827,6 @@ static void amdtp_stream_first_callback(struct fw_iso_context *context, cycle = compute_cycle_count(ctx_header[1]);
context->callback.sc = in_stream_callback; - if (s->flags & CIP_NO_HEADER) - s->handle_packet = handle_in_packet_without_header; - else - s->handle_packet = handle_in_packet; } else { cycle = compute_it_cycle(*ctx_header);
On Wed, 22 May 2019 16:17:02 +0200, Takashi Sakamoto wrote:
Hi,
In IR context of Linux FireWire subsystem, some quadlets of packet payload can be handled as a part of context header. As a result context payload can just include the rest of packet payload.
This patchset uses the mechanism to unify handlers of incoming packet for with-CIP and without-CIP headers.
Takashi Sakamoto (6): ALSA: firewire-lib: use clear name for variable of CIP header ALSA: firewire-lib: calculate the length of packet payload in packet handler ALSA: firewire-lib: compute pointer to payload buffer in context handler ALSA: firewire-lib: split helper function to check incoming CIP header ALSA: firewire-lib: use 16 bytes IR context header to separate CIP header ALSA: firewire-lib: unify packet handler for IR context
Applied all six patches now. Thanks.
Takashi
On Wed, 22 May 2019 16:17:02 +0200, Takashi Sakamoto wrote:
Hi,
In IR context of Linux FireWire subsystem, some quadlets of packet payload can be handled as a part of context header. As a result context payload can just include the rest of packet payload.
This patchset uses the mechanism to unify handlers of incoming packet for with-CIP and without-CIP headers.
Takashi Sakamoto (6): ALSA: firewire-lib: use clear name for variable of CIP header ALSA: firewire-lib: calculate the length of packet payload in packet handler ALSA: firewire-lib: compute pointer to payload buffer in context handler ALSA: firewire-lib: split helper function to check incoming CIP header ALSA: firewire-lib: use 16 bytes IR context header to separate CIP header ALSA: firewire-lib: unify packet handler for IR context
I already applied the previous patchset that had been submitted on Tuesday, and this seems conflicting.
Could you rebase and resubmit the additional fixes?
thanks,
Takashi
On Fri, 24 May 2019 07:58:47 +0200, Takashi Iwai wrote:
On Wed, 22 May 2019 16:17:02 +0200, Takashi Sakamoto wrote:
Hi,
In IR context of Linux FireWire subsystem, some quadlets of packet payload can be handled as a part of context header. As a result context payload can just include the rest of packet payload.
This patchset uses the mechanism to unify handlers of incoming packet for with-CIP and without-CIP headers.
Takashi Sakamoto (6): ALSA: firewire-lib: use clear name for variable of CIP header ALSA: firewire-lib: calculate the length of packet payload in packet handler ALSA: firewire-lib: compute pointer to payload buffer in context handler ALSA: firewire-lib: split helper function to check incoming CIP header ALSA: firewire-lib: use 16 bytes IR context header to separate CIP header ALSA: firewire-lib: unify packet handler for IR context
I already applied the previous patchset that had been submitted on Tuesday, and this seems conflicting.
Sorry, I replied to the wrong thread. I meant the new one 20190523151440.5127-1-o-takashi@sakamocchi.jp conflicting with the current for-next branch.
Takashi
Could you rebase and resubmit the additional fixes?
thanks,
Takashi
On Fri, May 24, 2019 at 08:04:06AM +0200, Takashi Iwai wrote:
I already applied the previous patchset that had been submitted on Tuesday, and this seems conflicting.
Sorry, I replied to the wrong thread. I meant the new one 20190523151440.5127-1-o-takashi@sakamocchi.jp conflicting with the current for-next branch.
It's the latest one:
[alsa-devel] [PATCH 0/4] ALSA: firewire-lib: unify handlers for outgoing packet https://mailman.alsa-project.org/pipermail/alsa-devel/2019-May/149708.html
I'll check them and resubmit after rebasing to current for-next.
Thanks
Takashi Sakamoto
participants (2)
-
Takashi Iwai -
Takashi Sakamoto