diff mbox

[05/39] firewire-lib: Add support for AMDTP in-stream and PCM capture

Message ID 1394016507-15761-6-git-send-email-o-takashi@sakamocchi.jp (mailing list archive)
State Superseded
Delegated to: Takashi Iwai
Headers show

Commit Message

Takashi Sakamoto March 5, 2014, 10:47 a.m. UTC
For capturing PCM, this commit adds the functionality to handle in-stream.
This is also applied for dual-wire mode.

Currently, capturing 32bit samples are supported.
---
 sound/firewire/amdtp.c | 218 +++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 201 insertions(+), 17 deletions(-)

Comments

Clemens Ladisch March 9, 2014, 8:37 p.m. UTC | #1
Takashi Sakamoto wrote:
> For capturing PCM, this commit adds the functionality to handle in-stream.
> This is also applied for dual-wire mode.

> +++ b/sound/firewire/amdtp.c
> @@ -200,16 +208,26 @@ void amdtp_stream_set_pcm_format(struct amdtp_stream *s,
> +		if (s->direction == AMDTP_OUT_STREAM) {
> +			if (s->dual_wire)
> +				s->transfer_samples = amdtp_write_s32_dualwire;
> +			else
> +				s->transfer_samples = amdtp_write_s32;
> +		} else if (s->dual_wire) {
> +			s->transfer_samples = amdtp_read_s32_dualwire;
> +		} else {
> +			s->transfer_samples = amdtp_read_s32;
> +		}

Please format the if/else branches in the same way.

> +static void handle_in_packet(struct amdtp_stream *s,
> ...
> +	/* ignore empty CIP packet or NO-DATA AMDTP packet */
> +	if ((payload_quadlets < 3) ||
> +	    (((cip_header[1] & CIP_FDF_MASK) >> CIP_FDF_SFC_SHIFT) ==
> +							AMDTP_FDF_NO_DATA))

(AMDTP_FDF_NO_DATA << CIP_FDF_SFC_SHIFT) would be a constant that could
be computed at compile time.

> +	/* check packet continuity */
> +	s->data_block_counter = (s->data_block_counter + data_blocks) & 0xff;
> +	if (s->data_block_counter != data_block_counter) {
> +		dev_info_ratelimited(&s->unit->device,
> +				     "Detect uncontinuity of CIP packets\n");
> +		s->data_block_counter = data_block_counter;
> +		return;

In practice, this error means that a packet was dropped.  I guess this
doesn't actually happen too often, but eventually, we should handle
this either by inserting some samples or by stopping the stream.


Regards,
Clemens
Takashi Sakamoto March 10, 2014, 3:55 a.m. UTC | #2
(Mar 10 2014 05:37), Clemens Ladisch wrote:
> Please format the if/else branches in the same way.

Do you mean these codes should be like this?:
###
if (s->direction == AMDTP_OUT_STREAM) {
	if (s->dual_wire)
		s->transfer_samples = amdtp_write_s32_dualwire;
	else
		s->transfer_samples = amdtp_write_s32;
} else {
	if (s->dual_wire)
		s->transfer_samples = amdtp_read_s32_dualwire;
	else
		s->transfer_samples = amdtp_read_s32;
}
###

> (AMDTP_FDF_NO_DATA << CIP_FDF_SFC_SHIFT) would be a constant that could
> be computed at compile time.

It's nice, OK.

> In practice, this error means that a packet was dropped.

In theory, it's true.

But BeBoB based devices have a quirk related to this. (I forgot to 
mention about it.) Just after connecting, they transmit packets with 
0x00 for its DBC field.

Yamaha GO46:
[13803.762518] 00050000 9001ffff
[13803.762523] 00050000 9001ffff
[13803.762525] 00050000 9001ffff
[13803.762527] 00050000 9001ffff
...
[13804.737925] 00050000 90014a1d
[13804.737927] 00050008 90016387
[13804.737929] 00050010 9001ffff
...

 > I guess this doesn't actually happen too often, but eventually, we should
> handle this either by inserting some samples or by stopping the stream.

So if doing something for this state, we shouldn't stop streams (of 
cource, we can add a flag for BeBoB.) We should insert some zero PCM 
samples or zero MIDI messages.

But this idea forces PCM/MIDI functionality to work for these 
meaningless data. I think it's a waste of resources.


Thanks

Takashi Sakamoto
o-takashi@sakamocchi.jp
Takashi Sakamoto March 12, 2014, 2 a.m. UTC | #3
(Mar 10 2014 05:37), Clemens Ladisch wrote:
> In practice, this error means that a packet was dropped.  I guess this
> doesn't actually happen too often, but eventually, we should handle
> this either by inserting some samples or by stopping the stream.

For inserting some samples, I realize that there are some difficulties:
  - In non-blocking mode, assuming the number of PCM samples in dropped 
packets is difficult because specification just define '0 <= N <= 
SYT_INTERVAL' (N = the number of events in packet). Each device can have 
different sequence of N in packets.
  - In blocking mode, assuming the value of syt in dropped packets is 
difficult because it's related to timing.

So I give up to think about recovering from dropped packets and make 
firewire-lib stops streaming when detecting this state.

But this is not better for BeBoB based devices as I mentioned in 
previous message. So I'll add some flags and codes for BeBoB.


Regards

Takashi Sakamoto
o-takashi@sakamocchi.jp
Takashi Sakamoto March 14, 2014, 1:50 p.m. UTC | #4
Clemens,

> In practice, this error means that a packet was dropped.  I guess this
> doesn't actually happen too often, but eventually, we should handle
> this either by inserting some samples or by stopping the stream.

I'm working for this and I realize Fireworks has some quirks.

(I guess that I had experienced this last year then remove codes to 
check it and forget it now...)

Well, in IEC 61883-1, the value of dbc field corresponds to the first 
data block in a packet. And the value of dbc in a first packet is zero. 
So it also means count of transmitted data blocks (described in IEC 
61883-6).

But for Fireworks, the value of dbc field corresponds to the end data 
block in a packet. See two samples below.

BeBoB - M-Audio Firewire Solo:
34 02040010 900234B2
02 02040018 9002FFFF
34 02040018 900248B2
34 02040020 900260B2
34 02040028 900274B2
02 02040030 9002FFFF
34 02040030 900288B2
34 02040038 9002A0B2
34 02040040 9002B4B2
02 02040048 9002FFFF
34 02040048 9002C8B2
34 02040050 9002E0B2
34 02040058 9002F4B2
02 02040060 9002FFFF
34 02040060 900208B2

Fireworks - AudioFire4(firmware 5.21):
Payload CIP0 CIP1
3A 00070028 9001C704
02 00070028 90FFFFFF
3A 00070030 9001E06E
3A 00070038 9001F5D8
3A 00070040 90010B42
02 00070040 90FFFFFF
3A 00070048 900124AC
3A 00070050 90013A16
02 00070050 90FFFFFF
3A 00070058 90015380
3A 00070060 900168EA
02 00070060 90FFFFFF
3A 00070068 90018254

Let us focus on the value of dbc in empty packet. In BeBoB, the value of 
dbc in packets following to empty packet is the same as the value in the 
empty packet. In Fireworks, the value of dbc in packets following to 
empty packet is not the same as the value in the empty packet, however 
the value of dbc in packets followed by empty packet is the same as the 
value in the empty packet. This means dbc corresponds to the end of data 
block in current packet. I can see this quirk on 
AudioFire4/AudioFirePre8 with firmware version 4.8-5.8, so I guess all 
of Fireworks have this quirk. (quirk 1)

Additionally, firmware version 5.5 seems to have a quirk to report fix 
value (0x08) for dbc interval in all sampling rates. I confirm this on 
AudioFire4 and AudioFirePre8, so I guess all of Fireworks with firmware 
version 5.5 have this quirk. (quirk 2)

Furthermore, Firmware named 'AudioFire9' seems to have a quirk to report 
fix value (0x11) for dbs in all sampling rates instead of actual value. 
I confirm this on AudioFirePre8 with firmware version 4.8-5.8. 
AudioFire8 (since July 2009) uses the same firmware so I guess this 
model has also this quirk. (quirk 3)

And as I mentioned before, BeBoB has a quirk not to increment dbc in 
initial state of transmitting. (quirk 4)

If firewire-lib has codes for these quirks, it can check dbc continuity.
So I'll to add some flags for these issue into 'struct amdtp_stream', like:
struct amdtp_stream {
...
	bool tx_dbc_is_end_event;	// for quirk 1
	unsigned int tx_dbc_interval;	// for quirk 2
	bool tx_dbs_is_wrong;		// for quirk 3
	bool skip_initial_tx_packets;	// for quirk 4
...
};

But 'struct amdtp_stream' has 'flag' member. Should I use this member 
instead of adding new members?


Regards

Takashi Sakamoto
o-takashi@sakamocchi.jp
diff mbox

Patch

diff --git a/sound/firewire/amdtp.c b/sound/firewire/amdtp.c
index ca79cb4..4ebfd67 100644
--- a/sound/firewire/amdtp.c
+++ b/sound/firewire/amdtp.c
@@ -39,6 +39,7 @@ 
  * only "Clock-based rate control mode" is supported
  */
 #define AMDTP_FDF_AM824		(0 << (CIP_FDF_SFC_SHIFT + 3))
+#define AMDTP_FDF_NO_DATA	0xff
 #define AMDTP_DBS_MASK		0x00ff0000
 #define AMDTP_DBS_SHIFT		16
 #define AMDTP_DBC_MASK		0x000000ff
@@ -47,6 +48,7 @@ 
 #define INTERRUPT_INTERVAL	16
 #define QUEUE_LENGTH		48
 
+#define IN_PACKET_HEADER_SIZE	4
 #define OUT_PACKET_HEADER_SIZE	0
 
 static void pcm_period_tasklet(unsigned long data);
@@ -179,6 +181,12 @@  static void amdtp_write_s16_dualwire(struct amdtp_stream *s,
 static void amdtp_write_s32_dualwire(struct amdtp_stream *s,
 				     struct snd_pcm_substream *pcm,
 				     __be32 *buffer, unsigned int frames);
+static void amdtp_read_s32(struct amdtp_stream *s,
+			   struct snd_pcm_substream *pcm,
+			   __be32 *buffer, unsigned int frames);
+static void amdtp_read_s32_dualwire(struct amdtp_stream *s,
+				    struct snd_pcm_substream *pcm,
+				    __be32 *buffer, unsigned int frames);
 
 /**
  * amdtp_stream_set_pcm_format - set the PCM format
@@ -200,16 +208,26 @@  void amdtp_stream_set_pcm_format(struct amdtp_stream *s,
 		WARN_ON(1);
 		/* fall through */
 	case SNDRV_PCM_FORMAT_S16:
-		if (s->dual_wire)
-			s->transfer_samples = amdtp_write_s16_dualwire;
-		else
-			s->transfer_samples = amdtp_write_s16;
-		break;
+		if (s->direction == AMDTP_OUT_STREAM) {
+			if (s->dual_wire)
+				s->transfer_samples = amdtp_write_s16_dualwire;
+			else
+				s->transfer_samples = amdtp_write_s16;
+			break;
+		}
+		WARN_ON(1);
+		/* fall through */
 	case SNDRV_PCM_FORMAT_S32:
-		if (s->dual_wire)
-			s->transfer_samples = amdtp_write_s32_dualwire;
-		else
-			s->transfer_samples = amdtp_write_s32;
+		if (s->direction == AMDTP_OUT_STREAM) {
+			if (s->dual_wire)
+				s->transfer_samples = amdtp_write_s32_dualwire;
+			else
+				s->transfer_samples = amdtp_write_s32;
+		} else if (s->dual_wire) {
+			s->transfer_samples = amdtp_read_s32_dualwire;
+		} else {
+			s->transfer_samples = amdtp_read_s32;
+		}
 		break;
 	}
 }
@@ -420,6 +438,58 @@  static void amdtp_write_s16_dualwire(struct amdtp_stream *s,
 	}
 }
 
+static void amdtp_read_s32(struct amdtp_stream *s,
+			   struct snd_pcm_substream *pcm,
+			   __be32 *buffer, unsigned int frames)
+{
+	struct snd_pcm_runtime *runtime = pcm->runtime;
+	unsigned int remaining_frames, i, c;
+	u32 *dst;
+
+	dst  = (void *)runtime->dma_area +
+			frames_to_bytes(runtime, s->pcm_buffer_pointer);
+	remaining_frames = runtime->buffer_size - s->pcm_buffer_pointer;
+
+	for (i = 0; i < frames; ++i) {
+		for (c = 0; c < s->pcm_channels; ++c) {
+			*dst = be32_to_cpu(buffer[c]) << 8;
+			dst++;
+		}
+		buffer += s->data_block_quadlets;
+		if (--remaining_frames == 0)
+			dst = (void *)runtime->dma_area;
+	}
+}
+
+static void amdtp_read_s32_dualwire(struct amdtp_stream *s,
+				    struct snd_pcm_substream *pcm,
+				    __be32 *buffer, unsigned int frames)
+{
+	struct snd_pcm_runtime *runtime = pcm->runtime;
+	unsigned int channels, remaining_frames, i, c;
+	u32 *dst;
+
+	dst = (void *)runtime->dma_area +
+			frames_to_bytes(runtime, s->pcm_buffer_pointer);
+	remaining_frames = runtime->buffer_size - s->pcm_buffer_pointer;
+	channels = s->pcm_channels / 2;
+
+	for (i = 0; i < frames; ++i) {
+		for (c = 0; c < channels; ++c) {
+			*dst = be32_to_cpu(buffer[c * 2]) << 8;
+			dst++;
+		}
+		buffer += 1;
+		for (c = 0; c < channels; ++c) {
+			*dst = be32_to_cpu(buffer[c * 2]) << 8;
+			dst++;
+		}
+		buffer += s->data_block_quadlets - 1;
+		if (--remaining_frames == 0)
+			dst = (void *)runtime->dma_area;
+	}
+}
+
 static void amdtp_fill_pcm_silence(struct amdtp_stream *s,
 				   __be32 *buffer, unsigned int frames)
 {
@@ -505,6 +575,12 @@  static inline int queue_out_packet(struct amdtp_stream *s,
 			    payload_length, skip);
 }
 
+static inline int queue_in_packet(struct amdtp_stream *s)
+{
+	return queue_packet(s, IN_PACKET_HEADER_SIZE,
+			    amdtp_stream_get_max_payload(s), false);
+}
+
 static void handle_out_packet(struct amdtp_stream *s, unsigned int cycle)
 {
 	__be32 *buffer;
@@ -552,6 +628,67 @@  static void handle_out_packet(struct amdtp_stream *s, unsigned int cycle)
 		update_pcm_pointers(s, pcm, data_blocks);
 }
 
+static void handle_in_packet(struct amdtp_stream *s,
+			     unsigned int payload_quadlets,
+			     __be32 *buffer)
+{
+	u32 cip_header[2];
+	unsigned int data_block_quadlets, data_blocks, data_block_counter;
+	struct snd_pcm_substream *pcm;
+
+	cip_header[0] = be32_to_cpu(buffer[0]);
+	cip_header[1] = be32_to_cpu(buffer[1]);
+
+	/*
+	 * This module supports 'Two-quadlet CIP header with SYT field'.
+	 * For convinience, also check FMT field is AM824 or not.
+	 */
+	if (((cip_header[0] & CIP_EOH_MASK) == CIP_EOH) ||
+	    ((cip_header[1] & CIP_EOH_MASK) != CIP_EOH) ||
+	    ((cip_header[1] & CIP_FMT_MASK) != CIP_FMT_AM)) {
+		dev_info_ratelimited(&s->unit->device,
+				"Invalid CIP header for AMDTP: %08X:%08X\n",
+				cip_header[0], cip_header[1]);
+		return;
+	}
+
+	/* ignore empty CIP packet or NO-DATA AMDTP packet */
+	if ((payload_quadlets < 3) ||
+	    (((cip_header[1] & CIP_FDF_MASK) >> CIP_FDF_SFC_SHIFT) ==
+							AMDTP_FDF_NO_DATA))
+		return;
+
+	data_block_quadlets =
+			(cip_header[1] & AMDTP_DBS_MASK) >> AMDTP_DBS_SHIFT;
+	/* avoid division by zero */
+	if (data_block_quadlets == 0) {
+		dev_info_ratelimited(&s->unit->device,
+				 "Detect invalid value in dbs field: %08X\n",
+				 data_block_quadlets);
+		return;
+	}
+
+	data_blocks = (payload_quadlets - 2) / data_block_quadlets;
+	data_block_counter = cip_header[1] & AMDTP_DBC_MASK;
+
+	/* check packet continuity */
+	s->data_block_counter = (s->data_block_counter + data_blocks) & 0xff;
+	if (s->data_block_counter != data_block_counter) {
+		dev_info_ratelimited(&s->unit->device,
+				     "Detect uncontinuity of CIP packets\n");
+		s->data_block_counter = data_block_counter;
+		return;
+	}
+
+	buffer += 2;
+
+	pcm = ACCESS_ONCE(s->pcm);
+	if (pcm) {
+		s->transfer_samples(s, pcm, buffer, data_blocks);
+		update_pcm_pointers(s, pcm, data_blocks);
+	}
+}
+
 static void out_stream_callback(struct fw_iso_context *context, u32 cycle,
 				size_t header_length, void *header,
 				void *private_data)
@@ -571,6 +708,35 @@  static void out_stream_callback(struct fw_iso_context *context, u32 cycle,
 	fw_iso_context_queue_flush(s->context);
 }
 
+static void in_stream_callback(struct fw_iso_context *context, u32 cycle,
+			       size_t header_length, void *header,
+			       void *private_data)
+{
+	struct amdtp_stream *s = private_data;
+	unsigned int p, packets, data_quadlets;
+	__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;
+
+		/* The number of quadlets in this packet */
+		data_quadlets =
+			(be32_to_cpu(headers[p]) >> ISO_DATA_LENGTH_SHIFT) / 4;
+		/* handle each packet data */
+		handle_in_packet(s, data_quadlets, buffer);
+
+		if (queue_in_packet(s) < 0) {
+			amdtp_stream_pcm_abort(s);
+			return;
+		}
+	}
+
+	fw_iso_context_queue_flush(s->context);
+}
+
 /**
  * amdtp_stream_start - start transferring packets
  * @s: the AMDTP stream to start
@@ -595,7 +761,10 @@  int amdtp_stream_start(struct amdtp_stream *s, int channel, int speed)
 		[CIP_SFC_88200]  = {  0,   67 },
 		[CIP_SFC_176400] = {  0,   67 },
 	};
-	int err;
+	unsigned int header_size;
+	enum dma_data_direction dir;
+	fw_iso_callback_t cb;
+	int type, err;
 
 	mutex_lock(&s->mutex);
 
@@ -609,16 +778,26 @@  int amdtp_stream_start(struct amdtp_stream *s, int channel, int speed)
 	s->syt_offset_state = initial_state[s->sfc].syt_offset;
 	s->last_syt_offset = TICKS_PER_CYCLE;
 
+	/* initialize packet buffer */
+	if (s->direction == AMDTP_IN_STREAM) {
+		dir = DMA_FROM_DEVICE;
+		type = FW_ISO_CONTEXT_RECEIVE;
+		header_size = IN_PACKET_HEADER_SIZE;
+		cb = in_stream_callback;
+	} else {
+		dir = DMA_TO_DEVICE;
+		type = FW_ISO_CONTEXT_TRANSMIT;
+		header_size = OUT_PACKET_HEADER_SIZE;
+		cb = out_stream_callback;
+	}
 	err = iso_packets_buffer_init(&s->buffer, s->unit, QUEUE_LENGTH,
-				      amdtp_stream_get_max_payload(s),
-				      DMA_TO_DEVICE);
+				      amdtp_stream_get_max_payload(s), dir);
 	if (err < 0)
 		goto err_unlock;
 
 	s->context = fw_iso_context_create(fw_parent_device(s->unit)->card,
-					   FW_ISO_CONTEXT_TRANSMIT,
-					   channel, speed, 0,
-					   out_stream_callback, s);
+					   type, channel, speed, header_size,
+					   cb, s);
 	if (IS_ERR(s->context)) {
 		err = PTR_ERR(s->context);
 		if (err == -EBUSY)
@@ -631,13 +810,18 @@  int amdtp_stream_start(struct amdtp_stream *s, int channel, int speed)
 
 	s->packet_index = 0;
 	do {
-		err = queue_out_packet(s, 0, true);
+		if (s->direction == AMDTP_IN_STREAM)
+			err = queue_in_packet(s);
+		else
+			err = queue_out_packet(s, 0, true);
 		if (err < 0)
 			goto err_context;
 	} while (s->packet_index > 0);
 
+	/* NOTE: TAG1 matches CIP. This just affects in stream */
 	s->data_block_counter = 0;
-	err = fw_iso_context_start(s->context, -1, 0, 0);
+	err = fw_iso_context_start(s->context, -1, 0,
+				   FW_ISO_CONTEXT_MATCH_TAG1);
 	if (err < 0)
 		goto err_context;