[PATCH 0/6] ALSA: firewire-lib: code refactoring for processing rx packets

[PATCH 0/6] ALSA: firewire-lib: code refactoring for processing rx packets

[Takashi Sakamoto]

Hi,

This patchset refactors mainly for processing rx packets.

Except for devices handled by ALSA bebob and dice drivers, the devices
doesn't take care of the value of syt field of CIP header for playback
timing. To code it explicitly, the new flag is added in 1st patch.

Some of devices based on DICE ASICs supports two pairs of packet
streams. In the case, the sequence of rx packet in the streams should
be different independently so that each of them deliver timing
information from each peer. On the other hand, current implementation
pools the sequence in AMDTP domain, then the rx packets are processed
with the same pool. It is inconvenient and in 2nd patch the pool is
prepared per streams.

The other patches are for my future work.


Regards

Takashi Sakamoto (6):
  ALSA: firewire-lib: add flag to unaware of syt in CIP header
  ALSA: firewire-lib: pool sequence of packet in IT context
    independently
  ALSA: firewire-lib: code refactoring for generation of packet
    descriptors
  ALSA: firewire-lib: code refactoring for generation of syt sequence
  ALSA: firewire-lib: code refactoring for generation of data block
    sequence
  ALSA: firewire-lib: code refactoring for transfer delay

 sound/firewire/amdtp-stream.c        | 267 +++++++++++++--------------
 sound/firewire/amdtp-stream.h        |  33 ++--
 sound/firewire/motu/amdtp-motu.c     |   4 +-
 sound/firewire/tascam/amdtp-tascam.c |   6 +-
 4 files changed, 146 insertions(+), 164 deletions(-)

-- 
2.27.0

[PATCH 1/6] ALSA: firewire-lib: add flag to unaware of syt in CIP header

[Takashi Sakamoto]

Many devices are unaware of syt field in rx CIP for playback timing.

This commit adds a flag to cancel processing syt field. Actually,
syt calculation is required to decide the number of events per rx packet.
The flag put 0xffff to CIP header of rx packet. On the other hand,
The value of syt field in CIP header of tx packet is unavailable. The
sequence of packet descriptor for tx packet includes 0 for the offset
of syt field to avoid computation.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/amdtp-stream.c        | 27 ++++++++++-----------------
 sound/firewire/amdtp-stream.h        |  4 +++-
 sound/firewire/motu/amdtp-motu.c     |  4 +---
 sound/firewire/tascam/amdtp-tascam.c |  6 ++----
 4 files changed, 16 insertions(+), 25 deletions(-)

diff --git a/sound/firewire/amdtp-stream.c b/sound/firewire/amdtp-stream.c
index 3713188aac25..7e763f46e5a4 100644
--- a/sound/firewire/amdtp-stream.c
+++ b/sound/firewire/amdtp-stream.c
@@ -113,9 +113,6 @@ int amdtp_stream_init(struct amdtp_stream *s, struct fw_unit *unit,
 	s->fmt = fmt;
 	s->process_ctx_payloads = process_ctx_payloads;
 
-	if (dir == AMDTP_OUT_STREAM)
-		s->ctx_data.rx.syt_override = -1;
-
 	return 0;
 }
 EXPORT_SYMBOL(amdtp_stream_init);
@@ -638,7 +635,8 @@ static int check_cip_header(struct amdtp_stream *s, const __be32 *buf,
 
 	*data_block_counter = dbc;
 
-	*syt = cip_header[1] & CIP_SYT_MASK;
+	if (!(s->flags & CIP_UNAWARE_SYT))
+		*syt = cip_header[1] & CIP_SYT_MASK;
 
 	return 0;
 }
@@ -836,22 +834,23 @@ static void generate_pkt_descs(struct amdtp_stream *s, struct pkt_desc *descs,
 {
 	unsigned int dbc = s->data_block_counter;
 	unsigned int seq_index = s->ctx_data.rx.seq_index;
+	bool aware_syt = !(s->flags & CIP_UNAWARE_SYT);
 	int i;
 
 	for (i = 0; i < packets; ++i) {
 		struct pkt_desc *desc = descs + i;
 		unsigned int index = (s->packet_index + i) % s->queue_size;
 		const struct seq_desc *seq = seq_descs + seq_index;
-		unsigned int syt;
 
 		desc->cycle = compute_ohci_it_cycle(*ctx_header, s->queue_size);
 
-		syt = seq->syt_offset;
-		if (syt != CIP_SYT_NO_INFO) {
-			syt = compute_syt(syt, desc->cycle,
-					  s->ctx_data.rx.transfer_delay);
+		if (aware_syt && seq->syt_offset != CIP_SYT_NO_INFO) {
+			desc->syt = compute_syt(seq->syt_offset, desc->cycle,
+						s->ctx_data.rx.transfer_delay);
+		} else {
+			desc->syt = CIP_SYT_NO_INFO;
 		}
-		desc->syt = syt;
+
 		desc->data_blocks = seq->data_blocks;
 
 		if (s->flags & CIP_DBC_IS_END_EVENT)
@@ -924,21 +923,15 @@ static void process_rx_packets(struct fw_iso_context *context, u32 tstamp, size_
 
 	for (i = 0; i < packets; ++i) {
 		const struct pkt_desc *desc = s->pkt_descs + i;
-		unsigned int syt;
 		struct {
 			struct fw_iso_packet params;
 			__be32 header[CIP_HEADER_QUADLETS];
 		} template = { {0}, {0} };
 		bool sched_irq = false;
 
-		if (s->ctx_data.rx.syt_override < 0)
-			syt = desc->syt;
-		else
-			syt = s->ctx_data.rx.syt_override;
-
 		build_it_pkt_header(s, desc->cycle, &template.params, pkt_header_length,
 				    desc->data_blocks, desc->data_block_counter,
-				    syt, i);
+				    desc->syt, i);
 
 		if (s == s->domain->irq_target) {
 			event_count += desc->data_blocks;
diff --git a/sound/firewire/amdtp-stream.h b/sound/firewire/amdtp-stream.h
index b362a6499265..6c4d277dc0dd 100644
--- a/sound/firewire/amdtp-stream.h
+++ b/sound/firewire/amdtp-stream.h
@@ -35,6 +35,8 @@
  * @CIP_NO_HEADERS: a lack of headers in packets
  * @CIP_UNALIGHED_DBC: Only for in-stream. The value of dbc is not alighed to
  *	the value of current SYT_INTERVAL; e.g. initial value is not zero.
+ * @CIP_UNAWARE_SYT: For outgoing packet, the value in SYT field of CIP is 0xffff.
+ *	For incoming packet, the value in SYT field of CIP is not handled.
  */
 enum cip_flags {
 	CIP_NONBLOCKING		= 0x00,
@@ -48,6 +50,7 @@ enum cip_flags {
 	CIP_HEADER_WITHOUT_EOH	= 0x80,
 	CIP_NO_HEADER		= 0x100,
 	CIP_UNALIGHED_DBC	= 0x200,
+	CIP_UNAWARE_SYT		= 0x400,
 };
 
 /**
@@ -143,7 +146,6 @@ struct amdtp_stream {
 
 			// To generate CIP header.
 			unsigned int fdf;
-			int syt_override;
 
 			// To generate constant hardware IRQ.
 			unsigned int event_count;
diff --git a/sound/firewire/motu/amdtp-motu.c b/sound/firewire/motu/amdtp-motu.c
index 9ccde07d6295..18bf433f43b6 100644
--- a/sound/firewire/motu/amdtp-motu.c
+++ b/sound/firewire/motu/amdtp-motu.c
@@ -441,7 +441,7 @@ int amdtp_motu_init(struct amdtp_stream *s, struct fw_unit *unit,
 {
 	amdtp_stream_process_ctx_payloads_t process_ctx_payloads;
 	int fmt = CIP_FMT_MOTU;
-	int flags = CIP_BLOCKING;
+	unsigned int flags = CIP_BLOCKING | CIP_UNAWARE_SYT;
 	int err;
 
 	if (dir == AMDTP_IN_STREAM) {
@@ -479,8 +479,6 @@ int amdtp_motu_init(struct amdtp_stream *s, struct fw_unit *unit,
 	if (dir == AMDTP_OUT_STREAM) {
 		// Use fixed value for FDF field.
 		s->ctx_data.rx.fdf = MOTU_FDF_AM824;
-		// Not used.
-		s->ctx_data.rx.syt_override = 0xffff;
 	}
 
 	return 0;
diff --git a/sound/firewire/tascam/amdtp-tascam.c b/sound/firewire/tascam/amdtp-tascam.c
index f823a2ab3544..64d66a802545 100644
--- a/sound/firewire/tascam/amdtp-tascam.c
+++ b/sound/firewire/tascam/amdtp-tascam.c
@@ -228,6 +228,7 @@ int amdtp_tscm_init(struct amdtp_stream *s, struct fw_unit *unit,
 		    enum amdtp_stream_direction dir, unsigned int pcm_channels)
 {
 	amdtp_stream_process_ctx_payloads_t process_ctx_payloads;
+	unsigned int flags = CIP_NONBLOCKING | CIP_SKIP_DBC_ZERO_CHECK | CIP_UNAWARE_SYT;
 	struct amdtp_tscm *p;
 	unsigned int fmt;
 	int err;
@@ -240,8 +241,7 @@ int amdtp_tscm_init(struct amdtp_stream *s, struct fw_unit *unit,
 		process_ctx_payloads = process_it_ctx_payloads;
 	}
 
-	err = amdtp_stream_init(s, unit, dir,
-			CIP_NONBLOCKING | CIP_SKIP_DBC_ZERO_CHECK, fmt,
+	err = amdtp_stream_init(s, unit, dir, flags, fmt,
 			process_ctx_payloads, sizeof(struct amdtp_tscm));
 	if (err < 0)
 		return 0;
@@ -249,8 +249,6 @@ int amdtp_tscm_init(struct amdtp_stream *s, struct fw_unit *unit,
 	if (dir == AMDTP_OUT_STREAM) {
 		// Use fixed value for FDF field.
 		s->ctx_data.rx.fdf = 0x00;
-		// Not used.
-		s->ctx_data.rx.syt_override = 0x0000;
 	}
 
 	/* This protocol uses fixed number of data channels for PCM samples. */
-- 
2.27.0

[PATCH 2/6] ALSA: firewire-lib: pool sequence of packet in IT context independently

[Takashi Sakamoto]

Current implementation pools the sequence in AMDTP domain. This is convenient
regarding to memory usage and computation time, however inconvenient for the
devices such that several rx streams are expected to transfer timing
information independently.

This commit refactors to pool the sequence per rx packet stream.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/amdtp-stream.c | 150 ++++++++++++++--------------------
 sound/firewire/amdtp-stream.h |  22 ++---
 2 files changed, 72 insertions(+), 100 deletions(-)

diff --git a/sound/firewire/amdtp-stream.c b/sound/firewire/amdtp-stream.c
index 7e763f46e5a4..cf4fbbd18756 100644
--- a/sound/firewire/amdtp-stream.c
+++ b/sound/firewire/amdtp-stream.c
@@ -441,6 +441,30 @@ static unsigned int calculate_syt_offset(unsigned int *last_syt_offset,
 	return syt_offset;
 }
 
+static void pool_ideal_seq_descs(struct amdtp_stream *s, unsigned int count)
+{
+	unsigned int seq_tail = s->ctx_data.rx.seq.tail;
+	const unsigned int seq_size = s->ctx_data.rx.seq.size;
+	const unsigned int syt_interval = s->syt_interval;
+	const enum cip_sfc sfc = s->sfc;
+	const bool is_blocking = !!(s->flags & CIP_BLOCKING);
+	int i;
+
+	for (i = 0; i < count; ++i) {
+		struct seq_desc *desc = s->ctx_data.rx.seq.descs + seq_tail;
+
+		desc->syt_offset = calculate_syt_offset(&s->ctx_data.rx.last_syt_offset,
+					&s->ctx_data.rx.syt_offset_state, sfc);
+		desc->data_blocks = calculate_data_blocks(&s->ctx_data.rx.data_block_state,
+				is_blocking, desc->syt_offset == CIP_SYT_NO_INFO,
+				syt_interval, sfc);
+
+		seq_tail = (seq_tail + 1) % seq_size;
+	}
+
+	s->ctx_data.rx.seq.tail = seq_tail;
+}
+
 static void update_pcm_pointers(struct amdtp_stream *s,
 				struct snd_pcm_substream *pcm,
 				unsigned int frames)
@@ -833,14 +857,14 @@ static void generate_pkt_descs(struct amdtp_stream *s, struct pkt_desc *descs,
 			       unsigned int seq_size)
 {
 	unsigned int dbc = s->data_block_counter;
-	unsigned int seq_index = s->ctx_data.rx.seq_index;
+	unsigned int seq_head = s->ctx_data.rx.seq.head;
 	bool aware_syt = !(s->flags & CIP_UNAWARE_SYT);
 	int i;
 
 	for (i = 0; i < packets; ++i) {
 		struct pkt_desc *desc = descs + i;
 		unsigned int index = (s->packet_index + i) % s->queue_size;
-		const struct seq_desc *seq = seq_descs + seq_index;
+		const struct seq_desc *seq = seq_descs + seq_head;
 
 		desc->cycle = compute_ohci_it_cycle(*ctx_header, s->queue_size);
 
@@ -863,13 +887,13 @@ static void generate_pkt_descs(struct amdtp_stream *s, struct pkt_desc *descs,
 
 		desc->ctx_payload = s->buffer.packets[index].buffer;
 
-		seq_index = (seq_index + 1) % seq_size;
+		seq_head = (seq_head + 1) % seq_size;
 
 		++ctx_header;
 	}
 
 	s->data_block_counter = dbc;
-	s->ctx_data.rx.seq_index = seq_index;
+	s->ctx_data.rx.seq.head = seq_head;
 }
 
 static inline void cancel_stream(struct amdtp_stream *s)
@@ -911,8 +935,10 @@ static void process_rx_packets(struct fw_iso_context *context, u32 tstamp, size_
 	// Calculate the number of packets in buffer and check XRUN.
 	packets = header_length / sizeof(*ctx_header);
 
-	generate_pkt_descs(s, s->pkt_descs, ctx_header, packets, d->seq.descs,
-			   d->seq.size);
+	pool_ideal_seq_descs(s, packets);
+
+	generate_pkt_descs(s, s->pkt_descs, ctx_header, packets, s->ctx_data.rx.seq.descs,
+			   s->ctx_data.rx.seq.size);
 
 	process_ctx_payloads(s, s->pkt_descs, packets);
 
@@ -1152,51 +1178,6 @@ static void process_tx_packets_intermediately(struct fw_iso_context *context, u3
 	}
 }
 
-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 process_ctxs_in_domain(struct amdtp_domain *d)
 {
 	struct amdtp_stream *s;
@@ -1225,9 +1206,6 @@ static void irq_target_callback(struct fw_iso_context *context, u32 tstamp, size
 {
 	struct amdtp_stream *s = private_data;
 	struct amdtp_domain *d = s->domain;
-	unsigned int packets = header_length / sizeof(__be32);
-
-	pool_ideal_seq_descs(d, packets);
 
 	process_rx_packets(context, tstamp, header_length, header, private_data);
 	process_ctxs_in_domain(d);
@@ -1238,9 +1216,6 @@ static void irq_target_callback_intermediately(struct fw_iso_context *context, u
 {
 	struct amdtp_stream *s = private_data;
 	struct amdtp_domain *d = s->domain;
-	unsigned int packets = header_length / sizeof(__be32);
-
-	pool_ideal_seq_descs(d, packets);
 
 	process_rx_packets_intermediately(context, tstamp, header_length, header, private_data);
 	process_ctxs_in_domain(d);
@@ -1415,7 +1390,31 @@ static int amdtp_stream_start(struct amdtp_stream *s, int channel, int speed,
 		s->ctx_data.tx.max_ctx_payload_length = max_ctx_payload_size;
 		s->ctx_data.tx.ctx_header_size = ctx_header_size;
 	} else {
-		s->ctx_data.rx.seq_index = 0;
+		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 },
+		};
+
+		s->ctx_data.rx.seq.descs = kcalloc(queue_size, sizeof(*s->ctx_data.rx.seq.descs), GFP_KERNEL);
+		if (!s->ctx_data.rx.seq.descs)
+			goto err_context;
+		s->ctx_data.rx.seq.size = queue_size;
+		s->ctx_data.rx.seq.tail = 0;
+		s->ctx_data.rx.seq.head = 0;
+
+		entry = &initial_state[s->sfc];
+		s->ctx_data.rx.data_block_state = entry->data_block;
+		s->ctx_data.rx.syt_offset_state = entry->syt_offset;
+		s->ctx_data.rx.last_syt_offset = TICKS_PER_CYCLE;
+
 		s->ctx_data.rx.event_count = 0;
 	}
 
@@ -1471,6 +1470,8 @@ static int amdtp_stream_start(struct amdtp_stream *s, int channel, int speed,
 err_pkt_descs:
 	kfree(s->pkt_descs);
 err_context:
+	if (s->direction == AMDTP_OUT_STREAM)
+		kfree(s->ctx_data.rx.seq.descs);
 	fw_iso_context_destroy(s->context);
 	s->context = ERR_PTR(-1);
 err_buffer:
@@ -1581,7 +1582,8 @@ static void amdtp_stream_stop(struct amdtp_stream *s)
 	iso_packets_buffer_destroy(&s->buffer, s->unit);
 	kfree(s->pkt_descs);
 
-	s->callbacked = false;
+	if (s->direction == AMDTP_OUT_STREAM)
+		kfree(s->ctx_data.rx.seq.descs);
 
 	mutex_unlock(&s->mutex);
 }
@@ -1613,8 +1615,6 @@ int amdtp_domain_init(struct amdtp_domain *d)
 
 	d->events_per_period = 0;
 
-	d->seq.descs = NULL;
-
 	return 0;
 }
 EXPORT_SYMBOL_GPL(amdtp_domain_init);
@@ -1665,18 +1665,6 @@ EXPORT_SYMBOL_GPL(amdtp_domain_add_stream);
  */
 int amdtp_domain_start(struct amdtp_domain *d, unsigned int tx_init_skip_cycles)
 {
-	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 queue_size;
@@ -1705,17 +1693,6 @@ int amdtp_domain_start(struct amdtp_domain *d, unsigned int tx_init_skip_cycles)
 	queue_size = DIV_ROUND_UP(CYCLES_PER_SECOND * events_per_buffer,
 				  amdtp_rate_table[d->irq_target->sfc]);
 
-	d->seq.descs = kcalloc(queue_size, sizeof(*d->seq.descs), GFP_KERNEL);
-	if (!d->seq.descs)
-		return -ENOMEM;
-	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;
-
 	list_for_each_entry(s, &d->streams, list) {
 		unsigned int idle_irq_interval = 0;
 
@@ -1734,8 +1711,6 @@ int amdtp_domain_start(struct amdtp_domain *d, unsigned int tx_init_skip_cycles)
 error:
 	list_for_each_entry(s, &d->streams, list)
 		amdtp_stream_stop(s);
-	kfree(d->seq.descs);
-	d->seq.descs = NULL;
 	return err;
 }
 EXPORT_SYMBOL_GPL(amdtp_domain_start);
@@ -1760,8 +1735,5 @@ void amdtp_domain_stop(struct amdtp_domain *d)
 
 	d->events_per_period = 0;
 	d->irq_target = NULL;
-
-	kfree(d->seq.descs);
-	d->seq.descs = NULL;
 }
 EXPORT_SYMBOL_GPL(amdtp_domain_stop);
diff --git a/sound/firewire/amdtp-stream.h b/sound/firewire/amdtp-stream.h
index 6c4d277dc0dd..fc653fe95405 100644
--- a/sound/firewire/amdtp-stream.h
+++ b/sound/firewire/amdtp-stream.h
@@ -142,13 +142,23 @@ struct amdtp_stream {
 		struct {
 			// To calculate CIP data blocks and tstamp.
 			unsigned int transfer_delay;
-			unsigned int seq_index;
 
 			// To generate CIP header.
 			unsigned int fdf;
 
 			// To generate constant hardware IRQ.
 			unsigned int event_count;
+
+			struct {
+				struct seq_desc *descs;
+				unsigned int size;
+				unsigned int tail;
+				unsigned int head;
+			} seq;
+
+			unsigned int data_block_state;
+			unsigned int syt_offset_state;
+			unsigned int last_syt_offset;
 		} rx;
 	} ctx_data;
 
@@ -281,16 +291,6 @@ struct amdtp_domain {
 		unsigned int tx_start;
 		unsigned int rx_start;
 	} processing_cycle;
-
-	struct {
-		struct seq_desc *descs;
-		unsigned int size;
-		unsigned int tail;
-	} seq;
-
-	unsigned int data_block_state;
-	unsigned int syt_offset_state;
-	unsigned int last_syt_offset;
 };
 
 int amdtp_domain_init(struct amdtp_domain *d);
-- 
2.27.0

[PATCH 3/6] ALSA: firewire-lib: code refactoring for generation of packet descriptors

[Takashi Sakamoto]

This commit refactors the arguments of helper function to generate the
descriptors of packet.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/amdtp-stream.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/sound/firewire/amdtp-stream.c b/sound/firewire/amdtp-stream.c
index cf4fbbd18756..354512a350b7 100644
--- a/sound/firewire/amdtp-stream.c
+++ b/sound/firewire/amdtp-stream.c
@@ -851,11 +851,11 @@ static unsigned int compute_syt(unsigned int syt_offset, unsigned int cycle,
 	return syt & CIP_SYT_MASK;
 }
 
-static void generate_pkt_descs(struct amdtp_stream *s, struct pkt_desc *descs,
-			       const __be32 *ctx_header, unsigned int packets,
-			       const struct seq_desc *seq_descs,
-			       unsigned int seq_size)
+static void generate_pkt_descs(struct amdtp_stream *s, const __be32 *ctx_header, unsigned int packets)
 {
+	struct pkt_desc *descs = s->pkt_descs;
+	const struct seq_desc *seq_descs = s->ctx_data.rx.seq.descs;
+	const unsigned int seq_size = s->ctx_data.rx.seq.size;
 	unsigned int dbc = s->data_block_counter;
 	unsigned int seq_head = s->ctx_data.rx.seq.head;
 	bool aware_syt = !(s->flags & CIP_UNAWARE_SYT);
@@ -937,8 +937,7 @@ static void process_rx_packets(struct fw_iso_context *context, u32 tstamp, size_
 
 	pool_ideal_seq_descs(s, packets);
 
-	generate_pkt_descs(s, s->pkt_descs, ctx_header, packets, s->ctx_data.rx.seq.descs,
-			   s->ctx_data.rx.seq.size);
+	generate_pkt_descs(s, ctx_header, packets);
 
 	process_ctx_payloads(s, s->pkt_descs, packets);
 
-- 
2.27.0

[PATCH 4/6] ALSA: firewire-lib: code refactoring for generation of syt sequence

[Takashi Sakamoto]

This commit dissolves sequence generator in terms of syt offsets for
packet.

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

diff --git a/sound/firewire/amdtp-stream.c b/sound/firewire/amdtp-stream.c
index 354512a350b7..77ae75e79a43 100644
--- a/sound/firewire/amdtp-stream.c
+++ b/sound/firewire/amdtp-stream.c
@@ -441,8 +441,30 @@ static unsigned int calculate_syt_offset(unsigned int *last_syt_offset,
 	return syt_offset;
 }
 
+static void pool_ideal_syt_offsets(struct amdtp_stream *s, struct seq_desc *descs,
+				   const unsigned int seq_size, unsigned int seq_tail,
+				   unsigned int count)
+{
+	const enum cip_sfc sfc = s->sfc;
+	unsigned int last = s->ctx_data.rx.last_syt_offset;
+	unsigned int state = s->ctx_data.rx.syt_offset_state;
+	int i;
+
+	for (i = 0; i < count; ++i) {
+		struct seq_desc *desc = descs + seq_tail;
+
+		desc->syt_offset = calculate_syt_offset(&last, &state, sfc);
+
+		seq_tail = (seq_tail + 1) % seq_size;
+	}
+
+	s->ctx_data.rx.last_syt_offset = last;
+	s->ctx_data.rx.syt_offset_state = state;
+}
+
 static void pool_ideal_seq_descs(struct amdtp_stream *s, unsigned int count)
 {
+	struct seq_desc *descs = s->ctx_data.rx.seq.descs;
 	unsigned int seq_tail = s->ctx_data.rx.seq.tail;
 	const unsigned int seq_size = s->ctx_data.rx.seq.size;
 	const unsigned int syt_interval = s->syt_interval;
@@ -450,11 +472,11 @@ static void pool_ideal_seq_descs(struct amdtp_stream *s, unsigned int count)
 	const bool is_blocking = !!(s->flags & CIP_BLOCKING);
 	int i;
 
+	pool_ideal_syt_offsets(s, descs, seq_size, seq_tail, count);
+
 	for (i = 0; i < count; ++i) {
 		struct seq_desc *desc = s->ctx_data.rx.seq.descs + seq_tail;
 
-		desc->syt_offset = calculate_syt_offset(&s->ctx_data.rx.last_syt_offset,
-					&s->ctx_data.rx.syt_offset_state, sfc);
 		desc->data_blocks = calculate_data_blocks(&s->ctx_data.rx.data_block_state,
 				is_blocking, desc->syt_offset == CIP_SYT_NO_INFO,
 				syt_interval, sfc);
-- 
2.27.0

[PATCH 5/6] ALSA: firewire-lib: code refactoring for generation of data block sequence

[Takashi Sakamoto]

This commit dissolves sequence generator in terms of the number of data
blocks per packet.

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

diff --git a/sound/firewire/amdtp-stream.c b/sound/firewire/amdtp-stream.c
index 77ae75e79a43..26209513199a 100644
--- a/sound/firewire/amdtp-stream.c
+++ b/sound/firewire/amdtp-stream.c
@@ -357,26 +357,41 @@ void amdtp_stream_pcm_prepare(struct amdtp_stream *s)
 }
 EXPORT_SYMBOL(amdtp_stream_pcm_prepare);
 
-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)
+static void pool_blocking_data_blocks(struct amdtp_stream *s, struct seq_desc *descs,
+				      const unsigned int seq_size, unsigned int seq_tail,
+				      unsigned int count)
 {
-	unsigned int data_blocks;
+	const unsigned int syt_interval = s->syt_interval;
+	int i;
+
+	for (i = 0; i < count; ++i) {
+		struct seq_desc *desc = descs + seq_tail;
 
-	/* Blocking mode. */
-	if (is_blocking) {
-		/* This module generate empty packet for 'no data'. */
-		if (is_no_info)
-			data_blocks = 0;
+		if (desc->syt_offset != CIP_SYT_NO_INFO)
+			desc->data_blocks = syt_interval;
 		else
-			data_blocks = syt_interval;
-	/* Non-blocking mode. */
-	} else {
+			desc->data_blocks = 0;
+
+		seq_tail = (seq_tail + 1) % seq_size;
+	}
+}
+
+static void pool_ideal_nonblocking_data_blocks(struct amdtp_stream *s, struct seq_desc *descs,
+					       const unsigned int seq_size, unsigned int seq_tail,
+					       unsigned int count)
+{
+	const enum cip_sfc sfc = s->sfc;
+	unsigned int state = s->ctx_data.rx.data_block_state;
+	int i;
+
+	for (i = 0; i < count; ++i) {
+		struct seq_desc *desc = descs + seq_tail;
+
 		if (!cip_sfc_is_base_44100(sfc)) {
 			// Sample_rate / 8000 is an integer, and precomputed.
-			data_blocks = *data_block_state;
+			desc->data_blocks = state;
 		} else {
-			unsigned int phase = *data_block_state;
+			unsigned int phase = state;
 
 		/*
 		 * This calculates the number of data blocks per packet so that
@@ -388,18 +403,19 @@ static unsigned int calculate_data_blocks(unsigned int *data_block_state,
 		 */
 			if (sfc == CIP_SFC_44100)
 				/* 6 6 5 6 5 6 5 ... */
-				data_blocks = 5 + ((phase & 1) ^
-						   (phase == 0 || phase >= 40));
+				desc->data_blocks = 5 + ((phase & 1) ^ (phase == 0 || phase >= 40));
 			else
 				/* 12 11 11 11 11 ... or 23 22 22 22 22 ... */
-				data_blocks = 11 * (sfc >> 1) + (phase == 0);
+				desc->data_blocks = 11 * (sfc >> 1) + (phase == 0);
 			if (++phase >= (80 >> (sfc >> 1)))
 				phase = 0;
-			*data_block_state = phase;
+			state = phase;
 		}
+
+		seq_tail = (seq_tail + 1) % seq_size;
 	}
 
-	return data_blocks;
+	s->ctx_data.rx.data_block_state = state;
 }
 
 static unsigned int calculate_syt_offset(unsigned int *last_syt_offset,
@@ -467,24 +483,15 @@ static void pool_ideal_seq_descs(struct amdtp_stream *s, unsigned int count)
 	struct seq_desc *descs = s->ctx_data.rx.seq.descs;
 	unsigned int seq_tail = s->ctx_data.rx.seq.tail;
 	const unsigned int seq_size = s->ctx_data.rx.seq.size;
-	const unsigned int syt_interval = s->syt_interval;
-	const enum cip_sfc sfc = s->sfc;
-	const bool is_blocking = !!(s->flags & CIP_BLOCKING);
-	int i;
 
 	pool_ideal_syt_offsets(s, descs, seq_size, seq_tail, count);
 
-	for (i = 0; i < count; ++i) {
-		struct seq_desc *desc = s->ctx_data.rx.seq.descs + seq_tail;
-
-		desc->data_blocks = calculate_data_blocks(&s->ctx_data.rx.data_block_state,
-				is_blocking, desc->syt_offset == CIP_SYT_NO_INFO,
-				syt_interval, sfc);
-
-		seq_tail = (seq_tail + 1) % seq_size;
-	}
+	if (s->flags & CIP_BLOCKING)
+		pool_blocking_data_blocks(s, descs, seq_size, seq_tail, count);
+	else
+		pool_ideal_nonblocking_data_blocks(s, descs, seq_size, seq_tail, count);
 
-	s->ctx_data.rx.seq.tail = seq_tail;
+	s->ctx_data.rx.seq.tail = (seq_tail + count) % seq_size;
 }
 
 static void update_pcm_pointers(struct amdtp_stream *s,
-- 
2.27.0

[PATCH 6/6] ALSA: firewire-lib: code refactoring for transfer delay

[Takashi Sakamoto]

In later commit, transfer delay is used in both IR and IT contexts. This
commit refactors regardless of transfer delay.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/amdtp-stream.c | 24 ++++++++----------------
 sound/firewire/amdtp-stream.h |  7 +++----
 2 files changed, 11 insertions(+), 20 deletions(-)

diff --git a/sound/firewire/amdtp-stream.c b/sound/firewire/amdtp-stream.c
index 26209513199a..6dceb8cd6e0c 100644
--- a/sound/firewire/amdtp-stream.c
+++ b/sound/firewire/amdtp-stream.c
@@ -294,17 +294,11 @@ int amdtp_stream_set_parameters(struct amdtp_stream *s, unsigned int rate,
 	s->syt_interval = amdtp_syt_intervals[sfc];
 
 	// default buffering in the device.
-	if (s->direction == AMDTP_OUT_STREAM) {
-		s->ctx_data.rx.transfer_delay =
-					TRANSFER_DELAY_TICKS - TICKS_PER_CYCLE;
-
-		if (s->flags & CIP_BLOCKING) {
-			// additional buffering needed to adjust for no-data
-			// packets.
-			s->ctx_data.rx.transfer_delay +=
-				TICKS_PER_SECOND * s->syt_interval / rate;
-		}
-	}
+	s->transfer_delay = TRANSFER_DELAY_TICKS - TICKS_PER_CYCLE;
+
+	// additional buffering needed to adjust for no-data packets.
+	if (s->flags & CIP_BLOCKING)
+		s->transfer_delay += TICKS_PER_SECOND * s->syt_interval / rate;
 
 	return 0;
 }
@@ -897,12 +891,10 @@ static void generate_pkt_descs(struct amdtp_stream *s, const __be32 *ctx_header,
 
 		desc->cycle = compute_ohci_it_cycle(*ctx_header, s->queue_size);
 
-		if (aware_syt && seq->syt_offset != CIP_SYT_NO_INFO) {
-			desc->syt = compute_syt(seq->syt_offset, desc->cycle,
-						s->ctx_data.rx.transfer_delay);
-		} else {
+		if (aware_syt && seq->syt_offset != CIP_SYT_NO_INFO)
+			desc->syt = compute_syt(seq->syt_offset, desc->cycle, s->transfer_delay);
+		else
 			desc->syt = CIP_SYT_NO_INFO;
-		}
 
 		desc->data_blocks = seq->data_blocks;
 
diff --git a/sound/firewire/amdtp-stream.h b/sound/firewire/amdtp-stream.h
index fc653fe95405..467d5021624b 100644
--- a/sound/firewire/amdtp-stream.h
+++ b/sound/firewire/amdtp-stream.h
@@ -140,15 +140,13 @@ struct amdtp_stream {
 			unsigned int dbc_interval;
 		} tx;
 		struct {
-			// To calculate CIP data blocks and tstamp.
-			unsigned int transfer_delay;
-
 			// To generate CIP header.
 			unsigned int fdf;
 
 			// To generate constant hardware IRQ.
 			unsigned int event_count;
 
+			// To calculate CIP data blocks and tstamp.
 			struct {
 				struct seq_desc *descs;
 				unsigned int size;
@@ -169,7 +167,8 @@ struct amdtp_stream {
 	unsigned int sph;
 	unsigned int fmt;
 
-	/* Internal flags. */
+	// Internal flags.
+	unsigned int transfer_delay;
 	enum cip_sfc sfc;
 	unsigned int syt_interval;
 
-- 
2.27.0

Re: [PATCH 0/6] ALSA: firewire-lib: code refactoring for processing rx packets

[Takashi Iwai]

On Sat, 22 May 2021 03:32:57 +0200,
Takashi Sakamoto wrote:
> 
> Hi,
> 
> This patchset refactors mainly for processing rx packets.
> 
> Except for devices handled by ALSA bebob and dice drivers, the devices
> doesn't take care of the value of syt field of CIP header for playback
> timing. To code it explicitly, the new flag is added in 1st patch.
> 
> Some of devices based on DICE ASICs supports two pairs of packet
> streams. In the case, the sequence of rx packet in the streams should
> be different independently so that each of them deliver timing
> information from each peer. On the other hand, current implementation
> pools the sequence in AMDTP domain, then the rx packets are processed
> with the same pool. It is inconvenient and in 2nd patch the pool is
> prepared per streams.
> 
> The other patches are for my future work.
> 
> 
> Regards
> 
> Takashi Sakamoto (6):
>   ALSA: firewire-lib: add flag to unaware of syt in CIP header
>   ALSA: firewire-lib: pool sequence of packet in IT context
>     independently
>   ALSA: firewire-lib: code refactoring for generation of packet
>     descriptors
>   ALSA: firewire-lib: code refactoring for generation of syt sequence
>   ALSA: firewire-lib: code refactoring for generation of data block
>     sequence
>   ALSA: firewire-lib: code refactoring for transfer delay

Thanks, applied all six patches now.


Takashi