Alsa-Devel Archive on lore.kernel.org
 help / color / Atom feed
From: Takashi Iwai <tiwai@suse.de>
To: Takashi Sakamoto <o-takashi@sakamocchi.jp>
Cc: alsa-devel@alsa-project.org, clemens@ladisch.de,
	ffado-devel@lists.sf.net
Subject: Re: [PATCH 09/39] firewire-lib: Add sort function for transmitted packet
Date: Fri, 28 Feb 2014 07:40:51 +0100
Message-ID: <s5h8usvn3ik.wl%tiwai@suse.de> (raw)
In-Reply-To: <1393558072-25926-10-git-send-email-o-takashi@sakamocchi.jp>

At Fri, 28 Feb 2014 12:27:22 +0900,
Takashi Sakamoto wrote:
> 
> This commit is to assure the continuity of timestamp in in-packets
> transmitted by devices.
> 
> Fireworks with firmware version 5.5 or former is a type of device which
> transmits packets with a bit disorder.
> 
> This commit add sorting of in-packets. When callback of receive-context
> is processed, the parameters of in-packets are stored in sort table and
> sorted by its value of data block counter. The sort algorism works faster
> in ordered sequence than in messy sequence. This is convinient for this
> purpose because the sequence is assumed not to be so messy. After sorting,
> packets are processed except for a last few packets in sort table. These
> packets are stored in buffer once and used in next cycle.
> 
> Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
> ---
>  sound/firewire/amdtp.c | 129 +++++++++++++++++++++++++++++++++++++++++++------
>  sound/firewire/amdtp.h |   4 ++
>  2 files changed, 119 insertions(+), 14 deletions(-)
> 
> diff --git a/sound/firewire/amdtp.c b/sound/firewire/amdtp.c
> index 9415992..3d943d1 100644
> --- a/sound/firewire/amdtp.c
> +++ b/sound/firewire/amdtp.c
> @@ -45,6 +45,7 @@
>  #define AMDTP_DBS_MASK		0x00ff0000
>  #define AMDTP_DBS_SHIFT		16
>  #define AMDTP_DBC_MASK		0x000000ff
> +#define DBC_THRESHOLD		(AMDTP_DBC_MASK / 2)
>  
>  /* TODO: make these configurable */
>  #define INTERRUPT_INTERVAL	16
> @@ -53,6 +54,13 @@
>  #define IN_PACKET_HEADER_SIZE	4
>  #define OUT_PACKET_HEADER_SIZE	0
>  
> +/* for re-ordering receive packets */
> +struct sort_table {
> +	unsigned int id;
> +	unsigned int dbc;
> +	unsigned int payload_size;
> +};
> +
>  static void pcm_period_tasklet(unsigned long data);
>  
>  /**
> @@ -77,6 +85,9 @@ int amdtp_stream_init(struct amdtp_stream *s, struct fw_unit *unit,
>  	s->callbacked = false;
>  	s->sync_slave = ERR_PTR(-1);
>  
> +	s->sort_table = NULL;
> +	s->left_packets = NULL;
> +
>  	return 0;
>  }
>  EXPORT_SYMBOL(amdtp_stream_init);
> @@ -735,6 +746,44 @@ static void handle_in_packet(struct amdtp_stream *s,
>  		update_pcm_pointers(s, pcm, data_blocks);
>  }
>  
> +#define SWAP(tbl, m, n) \
> +	t = tbl[n].id; \
> +	tbl[n].id = tbl[m].id; \
> +	tbl[m].id = t; \
> +	t = tbl[n].dbc; \
> +	tbl[n].dbc = tbl[m].dbc; \
> +	tbl[m].dbc = t; \
> +	t = tbl[n].payload_size; \
> +	tbl[n].payload_size = tbl[m].payload_size; \
> +	tbl[m].payload_size = t;

Why swap() macro can't be used instead?


> +static void packet_sort(struct sort_table *tbl, unsigned int len)
> +{
> +	unsigned int i, j, k, t;
> +
> +	i = 0;
> +	do {
> +		for (j = i + 1; j < len; j++) {
> +			if (((tbl[i].dbc > tbl[j].dbc) &&
> +			     (tbl[i].dbc - tbl[j].dbc < DBC_THRESHOLD))) {
> +				SWAP(tbl, i, j);
> +			} else if ((tbl[j].dbc > tbl[i].dbc) &&
> +				   (tbl[j].dbc - tbl[i].dbc >
> +							DBC_THRESHOLD)) {
> +				for (k = i; k > 0; k--) {
> +					if ((tbl[k].dbc > tbl[j].dbc) ||
> +					    (tbl[j].dbc - tbl[k].dbc >
> +							DBC_THRESHOLD)) {
> +						SWAP(tbl, j, k);
> +					}
> +					break;
> +				}
> +			}
> +			break;
> +		}
> +		i = j;
> +	} while (i < len);
> +}

You can use the standard sort() function (available in linux/sort.h).


> +
>  static void out_stream_callback(struct fw_iso_context *context, u32 cycle,
>  				size_t header_length, void *header,
>  				void *private_data)
> @@ -761,30 +810,66 @@ static void in_stream_callback(struct fw_iso_context *context, u32 cycle,
>  			       void *private_data)
>  {
>  	struct amdtp_stream *s = private_data;
> -	unsigned int p, packets, syt, data_quadlets;
> +	struct sort_table *entry, *tbl = s->sort_table;
> +	unsigned int i, j, k, index, packets, syt, remain_packets;
>  	__be32 *buffer, *headers = header;
>  
>  	/* The number of packets in buffer */
>  	packets = header_length / IN_PACKET_HEADER_SIZE;
>  
> -	for (p = 0; p < packets; p++) {
> -		buffer = s->buffer.packets[s->packet_index].buffer;
> +	/* Store into sort table and sort. */
> +	for (i = 0; i < packets; i++) {
> +		entry = &tbl[s->remain_packets + i];
> +		entry->id = i;
> +
> +		index = s->packet_index + i;
> +		if (index >= QUEUE_LENGTH)
> +			index -= QUEUE_LENGTH;
> +		buffer = s->buffer.packets[index].buffer;
> +		entry->dbc = be32_to_cpu(buffer[0]) & AMDTP_DBC_MASK;
>  
> -		/* The number of quadlets in this packet */
> -		data_quadlets =
> -			(be32_to_cpu(headers[p]) >> ISO_DATA_LENGTH_SHIFT) / 4;
> +		entry->payload_size = be32_to_cpu(headers[i]) >>
> +				      ISO_DATA_LENGTH_SHIFT;
> +	}
> +	packet_sort(tbl, packets + s->remain_packets);
>  
> -		/* Process sync slave stream */
> -		if ((s->flags & CIP_BLOCKING) &&
> -		    (s->flags & CIP_SYNC_TO_DEVICE) &&
> -		    s->sync_slave->callbacked) {
> -			syt = be32_to_cpu(buffer[1]) & CIP_SYT_MASK;
> -			handle_out_packet(s->sync_slave, syt);
> +	/*
> +	 * for convinience, tbl[i].id >= QUEUE_LENGTH is a label to identify
> +	 * previous packets in buffer.
> +	 */
> +	remain_packets = s->remain_packets;
> +	s->remain_packets = packets / 4;
> +	for (i = 0, j = 0, k = 0; i < remain_packets + packets; i++) {
> +		if (tbl[i].id < QUEUE_LENGTH) {
> +			index = s->packet_index + tbl[i].id;
> +			if (index >= QUEUE_LENGTH)
> +				index -= QUEUE_LENGTH;
> +			buffer = s->buffer.packets[index].buffer;
> +		} else {
> +			buffer = s->left_packets +
> +				 amdtp_stream_get_max_payload(s) * j++;
>  		}
>  
> -		/* handle each packet data */
> -		handle_in_packet(s, data_quadlets, buffer);
> +		if (i < remain_packets + packets - s->remain_packets) {
> +			/* Process sync slave stream */
> +			if ((s->flags & CIP_BLOCKING) &&
> +			    (s->flags & CIP_SYNC_TO_DEVICE) &&
> +			    s->sync_slave->callbacked) {
> +				syt = be32_to_cpu(buffer[1]) & CIP_SYT_MASK;
> +				handle_out_packet(s->sync_slave, syt);
> +			}
> +			handle_in_packet(s, tbl[i].payload_size / 4, buffer);
> +		} else {
> +			tbl[k].id = tbl[i].id + QUEUE_LENGTH;
> +			tbl[k].dbc = tbl[i].dbc;
> +			tbl[k].payload_size = tbl[i].payload_size;
> +			memcpy(s->left_packets +
> +					amdtp_stream_get_max_payload(s) * k++,
> +			       buffer, tbl[i].payload_size);
> +		}
> +	}
>  
> +	for (i = 0; i < packets; i++) {
>  		if (queue_in_packet(s) < 0) {
>  			amdtp_stream_pcm_abort(s);
>  			return;
> @@ -888,6 +973,17 @@ int amdtp_stream_start(struct amdtp_stream *s, int channel, int speed)
>  	if (err < 0)
>  		goto err_unlock;
>  
> +	/* for sorting transmitted packets */
> +	if (s->direction == AMDTP_IN_STREAM) {
> +		s->remain_packets = 0;
> +		s->sort_table = kzalloc(sizeof(struct sort_table) *
> +					QUEUE_LENGTH, GFP_KERNEL);
> +		if (s->sort_table == NULL)
> +			return -ENOMEM;
> +		s->left_packets = kzalloc(amdtp_stream_get_max_payload(s) *
> +					  QUEUE_LENGTH / 4, GFP_KERNEL);

NULL check missing?


thanks,

Takashi


> +	}
> +
>  	s->context = fw_iso_context_create(fw_parent_device(s->unit)->card,
>  					   type, channel, speed, header_size,
>  					   amdtp_stream_callback, s);
> @@ -986,6 +1082,11 @@ void amdtp_stream_stop(struct amdtp_stream *s)
>  	s->context = ERR_PTR(-1);
>  	iso_packets_buffer_destroy(&s->buffer, s->unit);
>  
> +	if (s->sort_table != NULL)
> +		kfree(s->sort_table);
> +	if (s->left_packets != NULL)
> +		kfree(s->left_packets);
> +
>  	s->callbacked = false;
>  
>  	mutex_unlock(&s->mutex);
> diff --git a/sound/firewire/amdtp.h b/sound/firewire/amdtp.h
> index efa2d25..d502646 100644
> --- a/sound/firewire/amdtp.h
> +++ b/sound/firewire/amdtp.h
> @@ -109,6 +109,10 @@ struct amdtp_stream {
>  	bool callbacked;
>  	wait_queue_head_t callback_wait;
>  	struct amdtp_stream *sync_slave;
> +
> +	void *sort_table;
> +	void *left_packets;
> +	unsigned int remain_packets;
>  };
>  
>  int amdtp_stream_init(struct amdtp_stream *s, struct fw_unit *unit,
> -- 
> 1.8.3.2
> 

  reply index

Thread overview: 74+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-02-28  3:27 [GIT PULL][PATCH 00/39] Enhancement of support for Firewire devices Takashi Sakamoto
2014-02-28  3:27 ` [PATCH 01/39] firewire-lib: Rename functions, structure, member for AMDTP Takashi Sakamoto
2014-02-28  3:27 ` [PATCH 02/39] firewire-lib: Add macros instead of fixed value " Takashi Sakamoto
2014-02-28  3:27 ` [PATCH 03/39] firewire-lib: Add 'direction' member to 'amdtp_stream' structure Takashi Sakamoto
2014-02-28  3:27 ` [PATCH 04/39] firewire-lib: Split some codes into functions to reuse for both streams Takashi Sakamoto
2014-02-28  3:27 ` [PATCH 05/39] firewire-lib: Add support for AMDTP in-stream and PCM capture Takashi Sakamoto
2014-02-28  3:27 ` [PATCH 06/39] firewire-lib: Add support for MIDI capture/playback Takashi Sakamoto
2014-02-28  3:27 ` [PATCH 07/39] firewire-lib: Give syt value as parameter to handle_out_packet() Takashi Sakamoto
2014-02-28  3:27 ` [PATCH 08/39] firewire-lib: Add support for duplex streams synchronization in blocking mode Takashi Sakamoto
2014-02-28  3:27 ` [PATCH 09/39] firewire-lib: Add sort function for transmitted packet Takashi Sakamoto
2014-02-28  6:40   ` Takashi Iwai [this message]
2014-02-28 14:31     ` Takashi Sakamoto
2014-02-28  3:27 ` [PATCH 10/39] firewire-lib: Add transfer delay to synchronized duplex streams Takashi Sakamoto
2014-02-28  3:27 ` [PATCH 11/39] firewire-lib: Add support for channel mapping Takashi Sakamoto
2014-02-28  3:27 ` [PATCH 12/39] firewire-lib: Rename macros, variables and functions for CMP Takashi Sakamoto
2014-02-28  3:27 ` [PATCH 13/39] firewire-lib: Add 'direction' member to 'cmp_connection' structure Takashi Sakamoto
2014-02-28  3:27 ` [PATCH 14/39] firewire-lib: Add handling output connection by CMP Takashi Sakamoto
2014-02-28  3:27 ` [PATCH 15/39] firewire-lib: Add a new function to check others' connection Takashi Sakamoto
2014-02-28  3:27 ` [PATCH 16/39] firewire-lib: Add some AV/C general commands Takashi Sakamoto
2014-02-28  3:27 ` [PATCH 17/39] firewire-lib: Add quirks for Fireworks Takashi Sakamoto
2014-02-28  3:27 ` [PATCH 18/39] firewire-lib: Add a fallback at RCODE_CANCELLED Takashi Sakamoto
2014-02-28 20:25   ` Stefan Richter
2014-02-28 20:39     ` Stefan Richter
2014-03-01  3:18     ` Takashi Sakamoto
2014-03-01 10:10       ` Stefan Richter
2014-03-01 12:22         ` Takashi Sakamoto
2014-03-01 14:20           ` Stefan Richter
2014-03-04  1:35             ` [FFADO-devel] " Jonathan Woithe
2014-03-04  8:33               ` Stefan Richter
2014-03-04  9:28                 ` Stefan Richter
2014-03-01 10:34   ` Stefan Richter
2014-03-01 12:23     ` Takashi Sakamoto
2014-02-28  3:27 ` [PATCH 19/39] fireworks: Add skelton for Fireworks based devices Takashi Sakamoto
2014-02-28  6:51   ` Takashi Iwai
2014-02-28 15:05     ` Takashi Sakamoto
2014-02-28 15:10       ` Takashi Iwai
2014-02-28 15:36         ` Takashi Sakamoto
2014-02-28  3:27 ` [PATCH 20/39] fireworks: Add transaction and some commands Takashi Sakamoto
2014-02-28  3:27 ` [PATCH 21/39] fireworks: Add connection and stream management Takashi Sakamoto
2014-02-28  3:27 ` [PATCH 22/39] fireworks: Add proc interface for debugging purpose Takashi Sakamoto
2014-02-28  6:54   ` Takashi Iwai
2014-03-01 13:17     ` Takashi Sakamoto
2014-03-03  9:01       ` Takashi Iwai
2014-03-04 14:10         ` Takashi Sakamoto
2014-02-28  3:27 ` [PATCH 23/39] fireworks: Add MIDI interface Takashi Sakamoto
2014-02-28  3:27 ` [PATCH 24/39] fireworks: Add PCM interface Takashi Sakamoto
2014-02-28  3:27 ` [PATCH 25/39] fireworks: Add hwdep interface Takashi Sakamoto
2014-02-28  3:27 ` [PATCH 26/39] fireworks: Add command/response functionality into " Takashi Sakamoto
2014-02-28  6:58   ` Takashi Iwai
2014-03-01 13:18     ` Takashi Sakamoto
2014-02-28  3:27 ` [PATCH 27/39] bebob: Add skelton for BeBoB based devices Takashi Sakamoto
2014-02-28  3:27 ` [PATCH 28/39] bebob: Add commands and connections/streams management Takashi Sakamoto
2014-02-28  3:27 ` [PATCH 29/39] bebob: Add proc interface for debugging purpose Takashi Sakamoto
2014-02-28  3:27 ` [PATCH 30/39] bebob: Add MIDI interface Takashi Sakamoto
2014-02-28  3:27 ` [PATCH 31/39] bebob: Add PCM interface Takashi Sakamoto
2014-02-28  3:27 ` [PATCH 32/39] bebob: Add hwdep interface Takashi Sakamoto
2014-02-28  3:27 ` [PATCH 33/39] bebob: Prepare for device specific operations Takashi Sakamoto
2014-02-28  3:27 ` [PATCH 34/39] bebob: Add support for Terratec PHASE, EWS series and Aureon Takashi Sakamoto
2014-02-28  3:27 ` [PATCH 35/39] bebob: Add support for Yamaha GO series Takashi Sakamoto
2014-02-28  3:27 ` [PATCH 36/39] bebob: Add support for Focusrite Saffire/SaffirePro series Takashi Sakamoto
2014-02-28  3:27 ` [PATCH 37/39] bebob: Add support for M-Audio usual Firewire series Takashi Sakamoto
2014-02-28  3:27 ` [PATCH 38/39] bebob: Send a cue to load firmware for M-Audio " Takashi Sakamoto
2014-02-28  3:27 ` [PATCH 39/39] bebob: Add support for M-Audio special " Takashi Sakamoto
2014-03-01 10:53   ` Stefan Richter
2014-03-01 13:06     ` Takashi Sakamoto
2014-03-01 14:32       ` Stefan Richter
2014-02-28  6:36 ` [GIT PULL][PATCH 00/39] Enhancement of support for Firewire devices Takashi Iwai
2014-03-01 13:14   ` Takashi Sakamoto
     [not found] <5316963F.1000206@sakamocchi.jp>
2014-03-05 10:47 ` [GIT PULL][PATCH 00/39 v2] " Takashi Sakamoto
2014-03-05 10:47   ` [PATCH 09/39] firewire-lib: Add sort function for transmitted packet Takashi Sakamoto
2014-03-09 21:07     ` Clemens Ladisch
2014-03-10 13:29       ` Takashi Sakamoto
2014-03-11  8:11         ` Clemens Ladisch
2014-03-12  1:23           ` Takashi Sakamoto
2014-03-12  7:21             ` Takashi Iwai

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=s5h8usvn3ik.wl%tiwai@suse.de \
    --to=tiwai@suse.de \
    --cc=alsa-devel@alsa-project.org \
    --cc=clemens@ladisch.de \
    --cc=ffado-devel@lists.sf.net \
    --cc=o-takashi@sakamocchi.jp \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Alsa-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/alsa-devel/0 alsa-devel/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 alsa-devel alsa-devel/ https://lore.kernel.org/alsa-devel \
		alsa-devel@alsa-project.org
	public-inbox-index alsa-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.alsa-project.alsa-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git