All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC][PATCH 00/11] digi00x: new driver for Digidesign 002/003 family
@ 2015-03-15 16:00 Takashi Sakamoto
  2015-03-15 16:00 ` [PATCH 01/11] ALSA: digi00x: add skelton for Digi 002/003 device driver Takashi Sakamoto
                   ` (10 more replies)
  0 siblings, 11 replies; 40+ messages in thread
From: Takashi Sakamoto @ 2015-03-15 16:00 UTC (permalink / raw)
  To: clemens, tiwai, robin, damien; +Cc: alsa-devel, ffado-devel

This patchset adds an experimental driver for Digidesign 002/003 family.
If you have any models, please test with this driver and report the result.

Currently, this driver correctly supports these major functionalities:
(as long as I tested)
 * PCM sample capture
 * MIDI messages capture
 * MIDI messages playback

And functionalities which need to be tested:
 * streaming with S/PDIF, ADAT and work clock source
 * MIDI device control

And includes a bug which should be fixed:
 * PCM sample playback

For a driver for these models, there was a preceding implementation by
Robin Gareus and Damien Zammit.
[alsa-devel] draft -- ALSA firewire + digi003
http://mailman.alsa-project.org/pipermail/alsa-devel/2013-January/058177.html

This patchset includes their implementation, called-as double-oh-three,
to multiplex PCM samples into CIP packet. As long as I test, their
implementation may includes any bugs and this driver still causes noisy
sound in several cases, while I confirm to get the same data block pattern
which Robin explains.
http://gareus.org/wiki/digi003

If you interested in this bug and tries to fix it, my packet dump may be
helpfull. Please see:
http://subversion.ffado.org/wiki/digi003rack
 * 96khz-24bit-1ch.log (27.5 kB)
 * 44khz-24bit-1ch.log (49.8 kB)
 * 44khz-24bit-2ch.log (49.8 kB)

And DKMS package is available on my github repository.
https://github.com/takaswie/snd-firewire-improve

Takashi Sakamoto (11):
  ALSA: digi00x: add skelton for Digi 002/003 device driver
  ALSA: digi00x: add streaming functionality
  ALSA: digi00x: add proc node for clock status
  ALSA: digi00x: add PCM functionality
  ALSA: digi00x: add MIDI functionality
  ALSA: digi00x: add hwdep interface
  ALSA: digi00x: support unknown asynchronous message
  ALSA: digi00x: support MIDI ports for device control
  ALSA: firewire-lib: allows to implement external MIDI callback
    function
  digi00x: improve MIDI capture/playback
  ALSA: digi00x: apply double-oh-three algorism to multiplex PCM samples

 include/uapi/sound/asound.h               |   3 +-
 include/uapi/sound/firewire.h             |   1 +
 sound/firewire/Kconfig                    |  10 +
 sound/firewire/Makefile                   |   1 +
 sound/firewire/amdtp.c                    |  30 ++-
 sound/firewire/amdtp.h                    |   5 +
 sound/firewire/digi00x/Makefile           |   3 +
 sound/firewire/digi00x/digi00x-hwdep.c    | 192 ++++++++++++++
 sound/firewire/digi00x/digi00x-midi.c     | 199 +++++++++++++++
 sound/firewire/digi00x/digi00x-pcm.c      | 336 +++++++++++++++++++++++++
 sound/firewire/digi00x/digi00x-proc.c     |  71 ++++++
 sound/firewire/digi00x/digi00x-protocol.c | 361 +++++++++++++++++++++++++++
 sound/firewire/digi00x/digi00x-stream.c   | 398 ++++++++++++++++++++++++++++++
 sound/firewire/digi00x/digi00x.c          | 179 ++++++++++++++
 sound/firewire/digi00x/digi00x.h          | 126 ++++++++++
 15 files changed, 1907 insertions(+), 8 deletions(-)
 create mode 100644 sound/firewire/digi00x/Makefile
 create mode 100644 sound/firewire/digi00x/digi00x-hwdep.c
 create mode 100644 sound/firewire/digi00x/digi00x-midi.c
 create mode 100644 sound/firewire/digi00x/digi00x-pcm.c
 create mode 100644 sound/firewire/digi00x/digi00x-proc.c
 create mode 100644 sound/firewire/digi00x/digi00x-protocol.c
 create mode 100644 sound/firewire/digi00x/digi00x-stream.c
 create mode 100644 sound/firewire/digi00x/digi00x.c
 create mode 100644 sound/firewire/digi00x/digi00x.h

-- 
2.1.0

^ permalink raw reply	[flat|nested] 40+ messages in thread

* [PATCH 01/11] ALSA: digi00x: add skelton for Digi 002/003 device driver
  2015-03-15 16:00 [RFC][PATCH 00/11] digi00x: new driver for Digidesign 002/003 family Takashi Sakamoto
@ 2015-03-15 16:00 ` Takashi Sakamoto
  2015-03-15 16:01 ` [PATCH 02/11] ALSA: digi00x: add streaming functionality Takashi Sakamoto
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 40+ messages in thread
From: Takashi Sakamoto @ 2015-03-15 16:00 UTC (permalink / raw)
  To: clemens, tiwai, robin, damien; +Cc: alsa-devel, ffado-devel

This commit adds a new driver for Digidesign 002/003 family. Currently
this driver just creates/removes card instance according to bus event.

Digidesign 002/003 family consists of:
 * Agere FW802B for IEEE 1394 PHY layer
 * PDI 1394L40 for IEEE 1394 LINK layer and IEC 61883 interface
 * ALTERA ACEX EP1K50 for IEC 61883 layer and DSP controller
 * ADSP-21065L for signal processing

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/Kconfig           |   8 +++
 sound/firewire/Makefile          |   1 +
 sound/firewire/digi00x/Makefile  |   2 +
 sound/firewire/digi00x/digi00x.c | 136 +++++++++++++++++++++++++++++++++++++++
 sound/firewire/digi00x/digi00x.h |  33 ++++++++++
 5 files changed, 180 insertions(+)
 create mode 100644 sound/firewire/digi00x/Makefile
 create mode 100644 sound/firewire/digi00x/digi00x.c
 create mode 100644 sound/firewire/digi00x/digi00x.h

diff --git a/sound/firewire/Kconfig b/sound/firewire/Kconfig
index ecec547..594b6d1 100644
--- a/sound/firewire/Kconfig
+++ b/sound/firewire/Kconfig
@@ -118,4 +118,12 @@ config SND_BEBOB
           To compile this driver as a module, choose M here: the module
           will be called snd-bebob.
 
+config SND_DIGI00X
+	tristate "Digidesign 002/003 family support"
+	help
+	 Say Y here to include support for Digidesign 002/003 family.
+
+         To compile this driver as a module, choose M here: the module
+         will be called snd-digi00x.
+
 endif # SND_FIREWIRE
diff --git a/sound/firewire/Makefile b/sound/firewire/Makefile
index 8b37f08..9b99ab2 100644
--- a/sound/firewire/Makefile
+++ b/sound/firewire/Makefile
@@ -11,3 +11,4 @@ obj-$(CONFIG_SND_ISIGHT) += snd-isight.o
 obj-$(CONFIG_SND_SCS1X) += snd-scs1x.o
 obj-$(CONFIG_SND_FIREWORKS) += fireworks/
 obj-$(CONFIG_SND_BEBOB) += bebob/
+obj-$(CONFIG_SND_DIGI00X) += digi00x/
diff --git a/sound/firewire/digi00x/Makefile b/sound/firewire/digi00x/Makefile
new file mode 100644
index 0000000..e30e233
--- /dev/null
+++ b/sound/firewire/digi00x/Makefile
@@ -0,0 +1,2 @@
+snd-digi00x-objs := digi00x.o
+obj-m += snd-digi00x.o
diff --git a/sound/firewire/digi00x/digi00x.c b/sound/firewire/digi00x/digi00x.c
new file mode 100644
index 0000000..6f427a2
--- /dev/null
+++ b/sound/firewire/digi00x/digi00x.c
@@ -0,0 +1,136 @@
+/*
+ * digi00x.c - a part of driver for Digidesign Digi 002/003 family
+ *
+ * Copyright (c) 2014-2015 Takashi Sakamoto
+ *
+ * Licensed under the terms of the GNU General Public License, version 2.
+ */
+
+#include "digi00x.h"
+
+MODULE_DESCRIPTION("Digidesign 002/003 Driver");
+MODULE_AUTHOR("Takashi Sakamoto <o-takashi@sakamocchi.jp>");
+MODULE_LICENSE("GPL v2");
+
+#define VENDOR_DIGIDESIGN	0x00a07e
+#define MODEL_DIGI00X		0x000002
+
+static int name_card(struct snd_dg00x *dg00x)
+{
+	struct fw_device *fw_dev = fw_parent_device(dg00x->unit);
+	char name[32] = {0};
+	char *model;
+	int err;
+
+	err = fw_csr_string(dg00x->unit->directory, CSR_MODEL, name,
+			    sizeof(name));
+	if (err < 0)
+		return err;
+
+	model = name;
+	if (model[0] == ' ')
+		model = strchr(model, ' ') + 1;
+
+	strcpy(dg00x->card->driver, "Digi00x");
+	strcpy(dg00x->card->shortname, model);
+	strcpy(dg00x->card->mixername, model);
+	snprintf(dg00x->card->longname, sizeof(dg00x->card->longname),
+		 "Digidesign %s, GUID %08x%08x at %s, S%d", model,
+		 cpu_to_be32(fw_dev->config_rom[3]),
+		 cpu_to_be32(fw_dev->config_rom[4]),
+		 dev_name(&dg00x->unit->device), 100 << fw_dev->max_speed);
+
+	return 0;
+}
+
+static void dg00x_card_free(struct snd_card *card)
+{
+	struct snd_dg00x *dg00x = card->private_data;
+
+	fw_unit_put(dg00x->unit);
+
+	mutex_destroy(&dg00x->mutex);
+}
+
+static int snd_dg00x_probe(struct fw_unit *unit,
+			   const struct ieee1394_device_id *entry)
+{
+	struct snd_card *card;
+	struct snd_dg00x *dg00x;
+	int err;
+
+	/* create card */
+	err = snd_card_new(&unit->device, -1, NULL, THIS_MODULE,
+			   sizeof(struct snd_dg00x), &card);
+	if (err < 0)
+		return err;
+	card->private_free = dg00x_card_free;
+
+	/* initialize myself */
+	dg00x = card->private_data;
+	dg00x->card = card;
+	dg00x->unit = fw_unit_get(unit);
+
+	mutex_init(&dg00x->mutex);
+	spin_lock_init(&dg00x->lock);
+
+	err = name_card(dg00x);
+	if (err < 0)
+		goto error;
+
+	err = snd_card_register(card);
+	if (err < 0)
+		goto error;
+
+	dev_set_drvdata(&unit->device, dg00x);
+
+	return err;
+error:
+	snd_card_free(card);
+	return err;
+}
+
+static void snd_dg00x_remove(struct fw_unit *unit)
+{
+	struct snd_dg00x *dg00x = dev_get_drvdata(&unit->device);
+
+	/* No need to wait for releasing card object in this context. */
+	snd_card_free_when_closed(dg00x->card);
+}
+
+
+static const struct ieee1394_device_id snd_dg00x_id_table[] = {
+	/* Both of 002/003 use the same ID. */
+	{
+		.match_flags = IEEE1394_MATCH_VENDOR_ID |
+			       IEEE1394_MATCH_MODEL_ID,
+		.vendor_id = VENDOR_DIGIDESIGN,
+		.model_id = MODEL_DIGI00X,
+	},
+	{}
+};
+MODULE_DEVICE_TABLE(ieee1394, snd_dg00x_id_table);
+
+static struct fw_driver dg00x_driver = {
+	.driver = {
+		.owner = THIS_MODULE,
+		.name = "snd-digi00x",
+		.bus = &fw_bus_type,
+	},
+	.probe    = snd_dg00x_probe,
+	.remove   = snd_dg00x_remove,
+	.id_table = snd_dg00x_id_table,
+};
+
+static int __init snd_dg00x_init(void)
+{
+	return driver_register(&dg00x_driver.driver);
+}
+
+static void __exit snd_dg00x_exit(void)
+{
+	driver_unregister(&dg00x_driver.driver);
+}
+
+module_init(snd_dg00x_init);
+module_exit(snd_dg00x_exit);
diff --git a/sound/firewire/digi00x/digi00x.h b/sound/firewire/digi00x/digi00x.h
new file mode 100644
index 0000000..59425cd
--- /dev/null
+++ b/sound/firewire/digi00x/digi00x.h
@@ -0,0 +1,33 @@
+/*
+ * digi00x.h - a part of driver for Digidesign Digi 002/003 family
+ *
+ * Copyright (c) 2014-2015 Takashi Sakamoto
+ *
+ * Licensed under the terms of the GNU General Public License, version 2.
+ */
+
+#ifndef SOUND_DIGI00X_H_INCLUDED
+#define SOUND_DIGI00X_H_INCLUDED
+
+#include <linux/compat.h>
+#include <linux/device.h>
+#include <linux/firewire.h>
+#include <linux/firewire-constants.h>
+#include <linux/module.h>
+#include <linux/mod_devicetable.h>
+#include <linux/delay.h>
+#include <linux/slab.h>
+
+#include <sound/core.h>
+#include <sound/initval.h>
+
+struct snd_dg00x {
+	struct snd_card *card;
+	struct fw_unit *unit;
+	int card_index;
+
+	struct mutex mutex;
+	spinlock_t lock;
+};
+
+#endif
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 02/11] ALSA: digi00x: add streaming functionality
  2015-03-15 16:00 [RFC][PATCH 00/11] digi00x: new driver for Digidesign 002/003 family Takashi Sakamoto
  2015-03-15 16:00 ` [PATCH 01/11] ALSA: digi00x: add skelton for Digi 002/003 device driver Takashi Sakamoto
@ 2015-03-15 16:01 ` Takashi Sakamoto
  2015-03-15 16:01 ` [PATCH 03/11] ALSA: digi00x: add proc node for clock status Takashi Sakamoto
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 40+ messages in thread
From: Takashi Sakamoto @ 2015-03-15 16:01 UTC (permalink / raw)
  To: clemens, tiwai, robin, damien; +Cc: alsa-devel, ffado-devel

This commit adds a functionality to manage streaming and to handle packets.

As long as seeing Digi 002, the device uses AMDTP stream in IEC 61883-1/6.
But the streaming is not controlled by CMP. It's controlled by writing to
certain addresses.

In Digi 002, several clock sources are available, while there're no
differences for packets in different clock sources. The value of SYT field
in transferred packets is always zero. Thus, streams in both direction don't
build synchronization. Tthis driver also ignores the synchronization.

And the device always requires received-stream to transmit packets, thus
the driver must transmit packets for both direction.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/Kconfig                  |   1 +
 sound/firewire/digi00x/Makefile         |   2 +-
 sound/firewire/digi00x/digi00x-stream.c | 351 ++++++++++++++++++++++++++++++++
 sound/firewire/digi00x/digi00x.c        |  16 ++
 sound/firewire/digi00x/digi00x.h        |  52 +++++
 5 files changed, 421 insertions(+), 1 deletion(-)
 create mode 100644 sound/firewire/digi00x/digi00x-stream.c

diff --git a/sound/firewire/Kconfig b/sound/firewire/Kconfig
index 594b6d1..6b9b0a1 100644
--- a/sound/firewire/Kconfig
+++ b/sound/firewire/Kconfig
@@ -120,6 +120,7 @@ config SND_BEBOB
 
 config SND_DIGI00X
 	tristate "Digidesign 002/003 family support"
+	select SND_FIREWIRE_LIB
 	help
 	 Say Y here to include support for Digidesign 002/003 family.
 
diff --git a/sound/firewire/digi00x/Makefile b/sound/firewire/digi00x/Makefile
index e30e233..fe8fceb 100644
--- a/sound/firewire/digi00x/Makefile
+++ b/sound/firewire/digi00x/Makefile
@@ -1,2 +1,2 @@
-snd-digi00x-objs := digi00x.o
+snd-digi00x-objs := digi00x.o digi00x-stream.o
 obj-m += snd-digi00x.o
diff --git a/sound/firewire/digi00x/digi00x-stream.c b/sound/firewire/digi00x/digi00x-stream.c
new file mode 100644
index 0000000..75f67b5
--- /dev/null
+++ b/sound/firewire/digi00x/digi00x-stream.c
@@ -0,0 +1,351 @@
+/*
+ * digi00x-stream.c - a part of driver for Digidesign Digi 002/003 family
+ *
+ * Copyright (c) 2014-2015 Takashi Sakamoto
+ *
+ * Licensed under the terms of the GNU General Public License, version 2.
+ */
+
+#include "digi00x.h"
+
+#define CALLBACK_TIMEOUT 500
+
+const unsigned int snd_dg00x_stream_rates[SND_DG00X_RATE_COUNT] = {
+	[0] = 44100,
+	[1] = 48000,
+	[2] = 88200,
+	[3] = 96000,
+};
+
+/* Multi Bit Linear Audio data channels for each sampling transfer frequency. */
+const unsigned int
+snd_dg00x_stream_mbla_data_channels[SND_DG00X_RATE_COUNT] = {
+	/* Analog/ADAT/SPDIF */
+	[0] = (8 + 8 + 2),
+	[1] = (8 + 8 + 2),
+	/* Analog/SPDIF */
+	[2] = (8 + 2),
+	[3] = (8 + 2),
+};
+
+int snd_dg00x_stream_get_rate(struct snd_dg00x *dg00x, unsigned int *rate)
+{
+	__be32 data;
+	int err;
+
+	err = snd_fw_transaction(dg00x->unit, TCODE_READ_QUADLET_REQUEST,
+				 0xffffe0000110ull, &data, sizeof(data), 0);
+	if (err < 0)
+		goto end;
+
+	data = be32_to_cpu(data) & 0x0f;
+	if (data >= ARRAY_SIZE(snd_dg00x_stream_rates)) {
+		err = -EIO;
+		goto end;
+	}
+
+	*rate = snd_dg00x_stream_rates[data];
+end:
+	return err;
+}
+
+int snd_dg00x_stream_set_rate(struct snd_dg00x *dg00x, unsigned int rate)
+{
+	__be32 data;
+	unsigned int i;
+
+	for (i = 0; i < ARRAY_SIZE(snd_dg00x_stream_rates); i++) {
+		if (rate == snd_dg00x_stream_rates[i])
+			break;
+	}
+	if (i == ARRAY_SIZE(snd_dg00x_stream_rates))
+		return -EIO;
+
+	data = cpu_to_be32(i);
+	return snd_fw_transaction(dg00x->unit, TCODE_WRITE_QUADLET_REQUEST,
+				  0xffffe0000110ull, &data, sizeof(data), 0);
+}
+
+int snd_dg00x_stream_get_clock(struct snd_dg00x *dg00x,
+			       enum snd_dg00x_clock *clock)
+{
+	__be32 data;
+	int err;
+
+	err = snd_fw_transaction(dg00x->unit, TCODE_READ_QUADLET_REQUEST,
+				 0xffffe0000118ull, &data, sizeof(data), 0);
+	if (err < 0)
+		return err;
+
+	*clock = be32_to_cpu(data) & 0x0f;
+	if (*clock >= ARRAY_SIZE(snd_dg00x_stream_rates))
+		err = -EIO;
+
+	return err;
+}
+
+int snd_dg00x_stream_get_optical_mode(struct snd_dg00x *dg00x,
+				      enum snd_dg00x_optical_mode *mode)
+{
+	__be32 data;
+	int err;
+
+	err = snd_fw_transaction(dg00x->unit, TCODE_READ_QUADLET_REQUEST,
+				 0xffffe000011c, &data, sizeof(data), 0);
+	if (err >= 0)
+		*mode = be32_to_cpu(data) & 0x01;
+
+	return err;
+}
+
+static void finish_session(struct snd_dg00x *dg00x)
+{
+	__be32 data = cpu_to_be32(0x00000003);
+
+	snd_fw_transaction(dg00x->unit, TCODE_WRITE_QUADLET_REQUEST,
+			   0xffffe0000004ull, &data, sizeof(data), 0);
+}
+
+static int begin_session(struct snd_dg00x *dg00x)
+{
+	__be32 data;
+	u32 curr;
+	int err;
+
+	err = snd_fw_transaction(dg00x->unit,
+				 TCODE_READ_QUADLET_REQUEST,
+				 0xffffe0000000ull,
+				 &data, sizeof(data), 0);
+	if (err < 0)
+		goto error;
+	curr = be32_to_cpu(data);
+
+	if (curr == 0)
+		curr = 2;
+
+	curr--;
+	while (curr > 0) {
+		data = cpu_to_be32(curr);
+		err = snd_fw_transaction(dg00x->unit,
+					 TCODE_WRITE_QUADLET_REQUEST,
+					 0xffffe0000004ull,
+					 &data, sizeof(data), 0);
+		if (err < 0)
+			goto error;
+
+		msleep(20);
+		curr--;
+	}
+
+	return 0;
+error:
+	finish_session(dg00x);
+	return err;
+}
+
+static void release_resources(struct snd_dg00x *dg00x)
+{
+	__be32 data = 0;
+
+	/* Unregister isochronous channels for both direction. */
+	snd_fw_transaction(dg00x->unit, TCODE_WRITE_QUADLET_REQUEST,
+			   0xffffe0000100ull, &data, sizeof(data), 0);
+
+	/* Release isochronous resources. */
+	fw_iso_resources_free(&dg00x->tx_resources);
+	fw_iso_resources_free(&dg00x->rx_resources);
+}
+
+static int keep_resources(struct snd_dg00x *dg00x, unsigned int rate)
+{
+	unsigned int i, p;
+	__be32 data;
+	int err;
+
+	/* Check sampling rate. */
+	for (i = 0; i < SND_DG00X_RATE_COUNT; i++) {
+		if (snd_dg00x_stream_rates[i] == rate)
+			break;
+	}
+	if (i == SND_DG00X_RATE_COUNT)
+		return -EINVAL;
+
+	/* Keep resources for out-stream. */
+	amdtp_stream_set_parameters(&dg00x->rx_stream, rate,
+				    snd_dg00x_stream_mbla_data_channels[i], 2);
+	err = fw_iso_resources_allocate(&dg00x->rx_resources,
+				amdtp_stream_get_max_payload(&dg00x->rx_stream),
+				fw_parent_device(dg00x->unit)->max_speed);
+	if (err < 0)
+		return err;
+
+	/* Keep resources for in-stream. */
+	amdtp_stream_set_parameters(&dg00x->tx_stream, rate,
+				    snd_dg00x_stream_mbla_data_channels[i], 1);
+	err = fw_iso_resources_allocate(&dg00x->tx_resources,
+				amdtp_stream_get_max_payload(&dg00x->tx_stream),
+				fw_parent_device(dg00x->unit)->max_speed);
+	if (err < 0)
+		goto error;
+
+	/* Register isochronous channels for both direction. */
+	data = cpu_to_be32((dg00x->tx_resources.channel << 16) |
+			   dg00x->rx_resources.channel);
+	err = snd_fw_transaction(dg00x->unit, TCODE_WRITE_QUADLET_REQUEST,
+				 0xffffe0000100ull, &data, sizeof(data), 0);
+	if (err < 0)
+		goto error;
+
+	/* The first data channel in a packet is for MIDI conformant data. */
+	for (p = 0; p < snd_dg00x_stream_mbla_data_channels[i]; p++) {
+		dg00x->rx_stream.pcm_positions[p] = p + 1;
+		dg00x->tx_stream.pcm_positions[p] = p + 1;
+	}
+	dg00x->rx_stream.midi_position = 0;
+	dg00x->tx_stream.midi_position = 0;
+
+	return 0;
+error:
+	release_resources(dg00x);
+	return err;
+}
+
+int snd_dg00x_stream_init_duplex(struct snd_dg00x *dg00x)
+{
+	int err;
+
+	/* For out-stream. */
+	err = fw_iso_resources_init(&dg00x->rx_resources, dg00x->unit);
+	if (err < 0)
+		return err;
+	err = amdtp_stream_init(&dg00x->rx_stream, dg00x->unit,
+				AMDTP_OUT_STREAM, CIP_NONBLOCKING);
+
+	/* For in-stream. */
+	err = fw_iso_resources_init(&dg00x->tx_resources, dg00x->unit);
+	if (err < 0)
+		return err;
+	err = amdtp_stream_init(&dg00x->tx_stream, dg00x->unit,
+				AMDTP_IN_STREAM,
+				CIP_BLOCKING | CIP_SKIP_INIT_DBC_CHECK);
+	if (err < 0)
+		amdtp_stream_destroy(&dg00x->rx_stream);
+
+	return err;
+}
+
+/*
+ * This function should be called before starting streams or after stopping
+ * streams.
+ */
+void snd_dg00x_stream_destroy_duplex(struct snd_dg00x *dg00x)
+{
+	amdtp_stream_destroy(&dg00x->rx_stream);
+	fw_iso_resources_destroy(&dg00x->rx_resources);
+
+	amdtp_stream_destroy(&dg00x->tx_stream);
+	fw_iso_resources_destroy(&dg00x->tx_resources);
+}
+
+int snd_dg00x_stream_start_duplex(struct snd_dg00x *dg00x, unsigned int rate)
+{
+	unsigned int curr_rate;
+	int err = 0;
+
+	if (dg00x->playback_substreams == 0 &&
+	    dg00x->capture_substreams == 0)
+		goto end;
+
+	/* Check current sampling rate. */
+	err = snd_dg00x_stream_get_rate(dg00x, &curr_rate);
+	if (err < 0)
+		goto error;
+	if ((curr_rate != rate) |
+	    amdtp_streaming_error(&dg00x->tx_stream) |
+	    amdtp_streaming_error(&dg00x->rx_stream)) {
+		finish_session(dg00x);
+
+		amdtp_stream_stop(&dg00x->tx_stream);
+		amdtp_stream_stop(&dg00x->rx_stream);
+		release_resources(dg00x);
+	}
+
+	/* No streams are transmitted without receiving a stream. */
+	if (!amdtp_stream_running(&dg00x->rx_stream)) {
+		err = snd_dg00x_stream_set_rate(dg00x, rate);
+		if (err < 0)
+			goto error;
+
+		err = keep_resources(dg00x, rate);
+		if (err < 0)
+			goto error;
+
+		err = begin_session(dg00x);
+		if (err < 0)
+			goto error;
+
+		err = amdtp_stream_start(&dg00x->rx_stream,
+				dg00x->rx_resources.channel,
+				fw_parent_device(dg00x->unit)->max_speed);
+		if (err < 0)
+			goto error;
+
+		if (!amdtp_stream_wait_callback(&dg00x->rx_stream,
+						CALLBACK_TIMEOUT)) {
+			err = -ETIMEDOUT;
+			goto error;
+		}
+	}
+
+	/*
+	 * The value of SYT field in transmitted packets is always 0x0000. Thus,
+	 * duplex streams with timestamp synchronization cannot be built.
+	 */
+	if (dg00x->capture_substreams > 0 &&
+	    !amdtp_stream_running(&dg00x->tx_stream)) {
+		err = amdtp_stream_start(&dg00x->tx_stream,
+				dg00x->tx_resources.channel,
+				fw_parent_device(dg00x->unit)->max_speed);
+		if (err < 0)
+			goto error;
+
+		if (!amdtp_stream_wait_callback(&dg00x->tx_stream,
+						CALLBACK_TIMEOUT)) {
+			err = -ETIMEDOUT;
+			goto error;
+		}
+	}
+end:
+	return err;
+error:
+	finish_session(dg00x);
+
+	amdtp_stream_stop(&dg00x->tx_stream);
+	amdtp_stream_stop(&dg00x->rx_stream);
+	release_resources(dg00x);
+
+	return err;
+}
+
+void snd_dg00x_stream_stop_duplex(struct snd_dg00x *dg00x)
+{
+	if (dg00x->capture_substreams > 0)
+		return;
+	amdtp_stream_stop(&dg00x->tx_stream);
+
+	if (dg00x->playback_substreams > 0)
+		return;
+	finish_session(dg00x);
+	amdtp_stream_stop(&dg00x->rx_stream);
+	release_resources(dg00x);
+}
+
+/* TODO: investigation. */
+void snd_dg00x_stream_update_duplex(struct snd_dg00x *dg00x)
+{
+	fw_iso_resources_update(&dg00x->tx_resources);
+	fw_iso_resources_update(&dg00x->rx_resources);
+
+	amdtp_stream_update(&dg00x->tx_stream);
+	amdtp_stream_update(&dg00x->rx_stream);
+}
diff --git a/sound/firewire/digi00x/digi00x.c b/sound/firewire/digi00x/digi00x.c
index 6f427a2..b82bce7 100644
--- a/sound/firewire/digi00x/digi00x.c
+++ b/sound/firewire/digi00x/digi00x.c
@@ -47,6 +47,8 @@ static void dg00x_card_free(struct snd_card *card)
 {
 	struct snd_dg00x *dg00x = card->private_data;
 
+	snd_dg00x_stream_destroy_duplex(dg00x);
+
 	fw_unit_put(dg00x->unit);
 
 	mutex_destroy(&dg00x->mutex);
@@ -78,6 +80,10 @@ static int snd_dg00x_probe(struct fw_unit *unit,
 	if (err < 0)
 		goto error;
 
+	err = snd_dg00x_stream_init_duplex(dg00x);
+	if (err < 0)
+		goto error;
+
 	err = snd_card_register(card);
 	if (err < 0)
 		goto error;
@@ -90,6 +96,15 @@ error:
 	return err;
 }
 
+static void snd_dg00x_update(struct fw_unit *unit)
+{
+	struct snd_dg00x *dg00x = dev_get_drvdata(&unit->device);
+
+	mutex_lock(&dg00x->mutex);
+	snd_dg00x_stream_update_duplex(dg00x);
+	mutex_unlock(&dg00x->mutex);
+}
+
 static void snd_dg00x_remove(struct fw_unit *unit)
 {
 	struct snd_dg00x *dg00x = dev_get_drvdata(&unit->device);
@@ -118,6 +133,7 @@ static struct fw_driver dg00x_driver = {
 		.bus = &fw_bus_type,
 	},
 	.probe    = snd_dg00x_probe,
+	.update   = snd_dg00x_update,
 	.remove   = snd_dg00x_remove,
 	.id_table = snd_dg00x_id_table,
 };
diff --git a/sound/firewire/digi00x/digi00x.h b/sound/firewire/digi00x/digi00x.h
index 59425cd..61cfd6e 100644
--- a/sound/firewire/digi00x/digi00x.h
+++ b/sound/firewire/digi00x/digi00x.h
@@ -21,6 +21,11 @@
 #include <sound/core.h>
 #include <sound/initval.h>
 
+#include "../lib.h"
+#include "../packets-buffer.h"
+#include "../iso-resources.h"
+#include "../amdtp.h"
+
 struct snd_dg00x {
 	struct snd_card *card;
 	struct fw_unit *unit;
@@ -28,6 +33,53 @@ struct snd_dg00x {
 
 	struct mutex mutex;
 	spinlock_t lock;
+
+	struct amdtp_stream tx_stream;
+	struct fw_iso_resources tx_resources;
+
+	struct amdtp_stream rx_stream;
+	struct fw_iso_resources rx_resources;
+
+	unsigned int playback_substreams;
+	unsigned int capture_substreams;
+};
+
+/* values for SND_DG00X_ADDR_OFFSET_RATE */
+enum snd_dg00x_rate {
+	SND_DG00X_RATE_44100 = 0,
+	SND_DG00X_RATE_48000,
+	SND_DG00X_RATE_88200,
+	SND_DG00X_RATE_96000,
+	SND_DG00X_RATE_COUNT,
 };
 
+/* values for SND_DG00X_ADDR_OFFSET_CLOCK */
+enum snd_dg00x_clock {
+	SND_DG00X_CLOCK_INTERNAL = 0,
+	SND_DG00X_CLOCK_SPDIF,
+	SND_DG00X_CLOCK_ADAT,
+	SND_DG00X_CLOCK_WORD,
+};
+
+/* values for SND_DG00X_ADDR_OFFSET_OPTICAL_MODE */
+enum snd_dg00x_optical_mode {
+	SND_DG00X_OPTICAL_MODE_ADAT = 0,
+	SND_DG00X_OPTICAL_MODE_SPDIF,
+};
+
+extern const unsigned int snd_dg00x_stream_rates[SND_DG00X_RATE_COUNT];
+extern const unsigned int
+snd_dg00x_stream_mbla_data_channels[SND_DG00X_RATE_COUNT];
+int snd_dg00x_stream_get_rate(struct snd_dg00x *dg00x, unsigned int *rate);
+int snd_dg00x_stream_set_rate(struct snd_dg00x *dg00x, unsigned int rate);
+int snd_dg00x_stream_get_clock(struct snd_dg00x *dg00x,
+			       enum snd_dg00x_clock *clock);
+int snd_dg00x_stream_get_optical_mode(struct snd_dg00x *dg00x,
+				      enum snd_dg00x_optical_mode *mode);
+int snd_dg00x_stream_init_duplex(struct snd_dg00x *dg00x);
+int snd_dg00x_stream_start_duplex(struct snd_dg00x *dg00x, unsigned int rate);
+void snd_dg00x_stream_stop_duplex(struct snd_dg00x *dg00x);
+void snd_dg00x_stream_update_duplex(struct snd_dg00x *dg00x);
+void snd_dg00x_stream_destroy_duplex(struct snd_dg00x *dg00x);
+
 #endif
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 03/11] ALSA: digi00x: add proc node for clock status
  2015-03-15 16:00 [RFC][PATCH 00/11] digi00x: new driver for Digidesign 002/003 family Takashi Sakamoto
  2015-03-15 16:00 ` [PATCH 01/11] ALSA: digi00x: add skelton for Digi 002/003 device driver Takashi Sakamoto
  2015-03-15 16:01 ` [PATCH 02/11] ALSA: digi00x: add streaming functionality Takashi Sakamoto
@ 2015-03-15 16:01 ` Takashi Sakamoto
  2015-03-15 16:01 ` [PATCH 04/11] ALSA: digi00x: add PCM functionality Takashi Sakamoto
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 40+ messages in thread
From: Takashi Sakamoto @ 2015-03-15 16:01 UTC (permalink / raw)
  To: clemens, tiwai, robin, damien; +Cc: alsa-devel, ffado-devel

This commit adds proc node to check current clock status for debugging.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/digi00x/Makefile       |  2 +-
 sound/firewire/digi00x/digi00x-proc.c | 71 +++++++++++++++++++++++++++++++++++
 sound/firewire/digi00x/digi00x.c      |  2 +
 sound/firewire/digi00x/digi00x.h      |  2 +
 4 files changed, 76 insertions(+), 1 deletion(-)
 create mode 100644 sound/firewire/digi00x/digi00x-proc.c

diff --git a/sound/firewire/digi00x/Makefile b/sound/firewire/digi00x/Makefile
index fe8fceb..aaec823 100644
--- a/sound/firewire/digi00x/Makefile
+++ b/sound/firewire/digi00x/Makefile
@@ -1,2 +1,2 @@
-snd-digi00x-objs := digi00x.o digi00x-stream.o
+snd-digi00x-objs := digi00x.o digi00x-stream.o digi00x-proc.o
 obj-m += snd-digi00x.o
diff --git a/sound/firewire/digi00x/digi00x-proc.c b/sound/firewire/digi00x/digi00x-proc.c
new file mode 100644
index 0000000..29b7efe
--- /dev/null
+++ b/sound/firewire/digi00x/digi00x-proc.c
@@ -0,0 +1,71 @@
+/*
+ * digi00x-proc.c - a part of driver for Digidesign Digi 002/003 family
+ *
+ * Copyright (c) 2014-2015 Takashi Sakamoto
+ *
+ * Licensed under the terms of the GNU General Public License, version 2.
+ */
+
+#include "./digi00x.h"
+
+static void proc_read_clock(struct snd_info_entry *entry,
+			    struct snd_info_buffer *buf)
+{
+	static const char *const source_name[] = {
+		[SND_DG00X_CLOCK_INTERNAL] = "internal",
+		[SND_DG00X_CLOCK_SPDIF] = "s/pdif",
+		[SND_DG00X_CLOCK_ADAT] = "adat",
+		[SND_DG00X_CLOCK_WORD] = "word clock",
+	};
+	static const char *const optical_name[] = {
+		[SND_DG00X_OPTICAL_MODE_ADAT] = "adat",
+		[SND_DG00X_OPTICAL_MODE_SPDIF] = "s/pdif",
+	};
+	struct snd_dg00x *dg00x = entry->private_data;
+	unsigned int rate;
+	enum snd_dg00x_clock clock;
+	enum snd_dg00x_optical_mode mode;
+
+	if (snd_dg00x_stream_get_rate(dg00x, &rate) < 0)
+		return;
+	if (snd_dg00x_stream_get_clock(dg00x, &clock) < 0)
+		return;
+	if (snd_dg00x_stream_get_optical_mode(dg00x, &mode) < 0)
+		return;
+
+	snd_iprintf(buf, "Sampling Rate: %d\n", rate);
+	snd_iprintf(buf, "Clock Source: %s\n", source_name[clock]);
+	snd_iprintf(buf, "Optical mode: %s\n", optical_name[mode]);
+}
+
+void snd_dg00x_proc_init(struct snd_dg00x *dg00x)
+{
+	struct snd_info_entry *root, *entry;
+
+	/*
+	 * All nodes are automatically removed at snd_card_disconnect(),
+	 * by following to link list.
+	 */
+	root = snd_info_create_card_entry(dg00x->card, "firewire",
+					  dg00x->card->proc_root);
+	if (root == NULL)
+		return;
+
+	root->mode = S_IFDIR | S_IRUGO | S_IXUGO;
+	if (snd_info_register(root) < 0) {
+		snd_info_free_entry(root);
+		return;
+	}
+
+	entry = snd_info_create_card_entry(dg00x->card, "clock", root);
+	if (entry == NULL) {
+		snd_info_free_entry(root);
+		return;
+	}
+
+	snd_info_set_text_ops(entry, dg00x, proc_read_clock);
+	if (snd_info_register(entry) < 0) {
+		snd_info_free_entry(entry);
+		snd_info_free_entry(root);
+	}
+}
diff --git a/sound/firewire/digi00x/digi00x.c b/sound/firewire/digi00x/digi00x.c
index b82bce7..d9c9e14 100644
--- a/sound/firewire/digi00x/digi00x.c
+++ b/sound/firewire/digi00x/digi00x.c
@@ -84,6 +84,8 @@ static int snd_dg00x_probe(struct fw_unit *unit,
 	if (err < 0)
 		goto error;
 
+	snd_dg00x_proc_init(dg00x);
+
 	err = snd_card_register(card);
 	if (err < 0)
 		goto error;
diff --git a/sound/firewire/digi00x/digi00x.h b/sound/firewire/digi00x/digi00x.h
index 61cfd6e..dde039f 100644
--- a/sound/firewire/digi00x/digi00x.h
+++ b/sound/firewire/digi00x/digi00x.h
@@ -20,6 +20,7 @@
 
 #include <sound/core.h>
 #include <sound/initval.h>
+#include <sound/info.h>
 
 #include "../lib.h"
 #include "../packets-buffer.h"
@@ -82,4 +83,5 @@ void snd_dg00x_stream_stop_duplex(struct snd_dg00x *dg00x);
 void snd_dg00x_stream_update_duplex(struct snd_dg00x *dg00x);
 void snd_dg00x_stream_destroy_duplex(struct snd_dg00x *dg00x);
 
+void snd_dg00x_proc_init(struct snd_dg00x *dg00x);
 #endif
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 04/11] ALSA: digi00x: add PCM functionality
  2015-03-15 16:00 [RFC][PATCH 00/11] digi00x: new driver for Digidesign 002/003 family Takashi Sakamoto
                   ` (2 preceding siblings ...)
  2015-03-15 16:01 ` [PATCH 03/11] ALSA: digi00x: add proc node for clock status Takashi Sakamoto
@ 2015-03-15 16:01 ` Takashi Sakamoto
  2015-03-15 16:01 ` [PATCH 05/11] ALSA: digi00x: add MIDI functionality Takashi Sakamoto
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 40+ messages in thread
From: Takashi Sakamoto @ 2015-03-15 16:01 UTC (permalink / raw)
  To: clemens, tiwai, robin, damien; +Cc: alsa-devel, ffado-devel

As long as seeing Digi 002, any PCM samples are transferred by Multi Bit
Linear Audio data channel in AMDTP packet. There're no quirks.

On the other hand, for received packets, the device expects drivers to use
a certain algorism to multiplex PCM samples into packets.

This commit adds PCM functionality to transmit/receive PCM samples with
current ALSA AMDTP engine, thus PCM playback substream causes noisy sound.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/digi00x/Makefile      |   2 +-
 sound/firewire/digi00x/digi00x-pcm.c | 325 +++++++++++++++++++++++++++++++++++
 sound/firewire/digi00x/digi00x.c     |   4 +
 sound/firewire/digi00x/digi00x.h     |   4 +
 4 files changed, 334 insertions(+), 1 deletion(-)
 create mode 100644 sound/firewire/digi00x/digi00x-pcm.c

diff --git a/sound/firewire/digi00x/Makefile b/sound/firewire/digi00x/Makefile
index aaec823..f2ed472 100644
--- a/sound/firewire/digi00x/Makefile
+++ b/sound/firewire/digi00x/Makefile
@@ -1,2 +1,2 @@
-snd-digi00x-objs := digi00x.o digi00x-stream.o digi00x-proc.o
+snd-digi00x-objs := digi00x.o digi00x-stream.o digi00x-proc.o digi00x-pcm.o
 obj-m += snd-digi00x.o
diff --git a/sound/firewire/digi00x/digi00x-pcm.c b/sound/firewire/digi00x/digi00x-pcm.c
new file mode 100644
index 0000000..4522b38
--- /dev/null
+++ b/sound/firewire/digi00x/digi00x-pcm.c
@@ -0,0 +1,325 @@
+/*
+ * digi00x-pcm.c - a part of driver for Digidesign Digi 002/003 family
+ *
+ * Copyright (c) 2014-2015 Takashi Sakamoto
+ *
+ * Licensed under the terms of the GNU General Public License, version 2.
+ */
+
+#include "digi00x.h"
+
+static int hw_rule_rate(struct snd_pcm_hw_params *params,
+			struct snd_pcm_hw_rule *rule)
+{
+	struct snd_interval *r =
+		hw_param_interval(params, SNDRV_PCM_HW_PARAM_RATE);
+	const struct snd_interval *c =
+		hw_param_interval_c(params, SNDRV_PCM_HW_PARAM_CHANNELS);
+	struct snd_interval t = {
+		.min = UINT_MAX, .max = 0, .integer = 1,
+	};
+	unsigned int i;
+
+	for (i = 0; i < SND_DG00X_RATE_COUNT; i++) {
+		if (!snd_interval_test(c,
+				       snd_dg00x_stream_mbla_data_channels[i]))
+			continue;
+
+		t.min = min(t.min, snd_dg00x_stream_rates[i]);
+		t.max = max(t.max, snd_dg00x_stream_rates[i]);
+	}
+
+	return snd_interval_refine(r, &t);
+}
+
+static int hw_rule_channels(struct snd_pcm_hw_params *params,
+			    struct snd_pcm_hw_rule *rule)
+{
+	struct snd_interval *c =
+		hw_param_interval(params, SNDRV_PCM_HW_PARAM_CHANNELS);
+	const struct snd_interval *r =
+		hw_param_interval_c(params, SNDRV_PCM_HW_PARAM_RATE);
+	struct snd_interval t = {
+		.min = UINT_MAX, .max = 0, .integer = 1,
+	};
+	unsigned int i;
+
+	for (i = 0; i < SND_DG00X_RATE_COUNT; i++) {
+		if (!snd_interval_test(r, snd_dg00x_stream_rates[i]))
+			continue;
+
+		t.min = min(t.min, snd_dg00x_stream_mbla_data_channels[i]);
+		t.max = max(t.max, snd_dg00x_stream_mbla_data_channels[i]);
+	}
+
+	return snd_interval_refine(c, &t);
+}
+
+static int pcm_init_hw_params(struct snd_dg00x *dg00x,
+			      struct snd_pcm_substream *substream)
+{
+	static const struct snd_pcm_hardware hardware = {
+		.info = SNDRV_PCM_INFO_BATCH |
+			SNDRV_PCM_INFO_BLOCK_TRANSFER |
+			SNDRV_PCM_INFO_INTERLEAVED |
+			SNDRV_PCM_INFO_JOINT_DUPLEX |
+			SNDRV_PCM_INFO_MMAP |
+			SNDRV_PCM_INFO_MMAP_VALID,
+		.formats = SNDRV_PCM_FMTBIT_S32,
+		.rates = SNDRV_PCM_RATE_44100 |
+			 SNDRV_PCM_RATE_48000 |
+			 SNDRV_PCM_RATE_88200 |
+			 SNDRV_PCM_RATE_96000,
+		.rate_min = 44100,
+		.rate_max = 96000,
+		.channels_min = 10,
+		.channels_max = 18,
+		.period_bytes_min = 4 * 18,
+		.period_bytes_max = 4 * 18 * 2048,
+		.buffer_bytes_max = 4 * 18 * 2048 * 2,
+		.periods_min = 2,
+		.periods_max = UINT_MAX,
+	};
+	int err;
+
+	substream->runtime->hw = hardware;
+
+	err = snd_pcm_hw_rule_add(substream->runtime, 0,
+				  SNDRV_PCM_HW_PARAM_CHANNELS,
+				  hw_rule_channels, NULL,
+				  SNDRV_PCM_HW_PARAM_RATE, -1);
+	if (err < 0)
+		goto end;
+
+	err = snd_pcm_hw_rule_add(substream->runtime, 0,
+				  SNDRV_PCM_HW_PARAM_RATE,
+				  hw_rule_rate, NULL,
+				  SNDRV_PCM_HW_PARAM_CHANNELS, -1);
+	if (err < 0)
+		goto end;
+
+	if (substream->stream == SNDRV_PCM_STREAM_CAPTURE)
+		err = amdtp_stream_add_pcm_hw_constraints(&dg00x->tx_stream,
+							  substream->runtime);
+	else
+		err = amdtp_stream_add_pcm_hw_constraints(&dg00x->rx_stream,
+							  substream->runtime);
+end:
+	return err;
+}
+
+static int pcm_open(struct snd_pcm_substream *substream)
+{
+	struct snd_dg00x *dg00x = substream->private_data;
+	enum snd_dg00x_clock clock;
+	unsigned int rate;
+	int err;
+
+	err = pcm_init_hw_params(dg00x, substream);
+	if (err < 0)
+		return err;
+
+	err = snd_dg00x_stream_get_clock(dg00x, &clock);
+	if (clock != SND_DG00X_CLOCK_INTERNAL) {
+		err = snd_dg00x_stream_get_rate(dg00x, &rate);
+		if (err < 0)
+			return err;
+		substream->runtime->hw.rate_min = rate;
+		substream->runtime->hw.rate_max = rate;
+	}
+
+	snd_pcm_set_sync(substream);
+
+	return 0;
+}
+
+static int pcm_close(struct snd_pcm_substream *substream)
+{
+	return 0;
+}
+
+static int pcm_capture_hw_params(struct snd_pcm_substream *substream,
+				 struct snd_pcm_hw_params *hw_params)
+{
+	struct snd_dg00x *dg00x = substream->private_data;
+
+	if (substream->runtime->status->state == SNDRV_PCM_STATE_OPEN) {
+		mutex_lock(&dg00x->mutex);
+		dg00x->capture_substreams++;
+		mutex_unlock(&dg00x->mutex);
+	}
+	amdtp_stream_set_pcm_format(&dg00x->tx_stream,
+				    params_format(hw_params));
+	return snd_pcm_lib_alloc_vmalloc_buffer(substream,
+						params_buffer_bytes(hw_params));
+}
+static int pcm_playback_hw_params(struct snd_pcm_substream *substream,
+				  struct snd_pcm_hw_params *hw_params)
+{
+	struct snd_dg00x *dg00x = substream->private_data;
+
+	if (substream->runtime->status->state == SNDRV_PCM_STATE_OPEN) {
+		mutex_lock(&dg00x->mutex);
+		dg00x->playback_substreams++;
+		mutex_unlock(&dg00x->mutex);
+	}
+	amdtp_stream_set_pcm_format(&dg00x->rx_stream,
+				    params_format(hw_params));
+	return snd_pcm_lib_alloc_vmalloc_buffer(substream,
+						params_buffer_bytes(hw_params));
+}
+
+static int pcm_capture_hw_free(struct snd_pcm_substream *substream)
+{
+	struct snd_dg00x *dg00x = substream->private_data;
+
+	mutex_lock(&dg00x->mutex);
+
+	if (substream->runtime->status->state != SNDRV_PCM_STATE_OPEN)
+		dg00x->capture_substreams--;
+
+	snd_dg00x_stream_stop_duplex(dg00x);
+
+	mutex_unlock(&dg00x->mutex);
+
+	return snd_pcm_lib_free_vmalloc_buffer(substream);
+}
+static int pcm_playback_hw_free(struct snd_pcm_substream *substream)
+{
+	struct snd_dg00x *dg00x = substream->private_data;
+
+	mutex_lock(&dg00x->mutex);
+
+	if (substream->runtime->status->state != SNDRV_PCM_STATE_OPEN)
+		dg00x->playback_substreams--;
+
+	snd_dg00x_stream_stop_duplex(dg00x);
+
+	mutex_unlock(&dg00x->mutex);
+
+	return snd_pcm_lib_free_vmalloc_buffer(substream);
+}
+
+static int pcm_capture_prepare(struct snd_pcm_substream *substream)
+{
+	struct snd_dg00x *dg00x = substream->private_data;
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	int err;
+
+	mutex_lock(&dg00x->mutex);
+
+	err = snd_dg00x_stream_start_duplex(dg00x, runtime->rate);
+	if (err >= 0)
+		amdtp_stream_pcm_prepare(&dg00x->tx_stream);
+
+	mutex_unlock(&dg00x->mutex);
+
+	return err;
+}
+static int pcm_playback_prepare(struct snd_pcm_substream *substream)
+{
+	struct snd_dg00x *dg00x = substream->private_data;
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	int err;
+
+	mutex_lock(&dg00x->mutex);
+
+	err = snd_dg00x_stream_start_duplex(dg00x, runtime->rate);
+	if (err >= 0)
+		amdtp_stream_pcm_prepare(&dg00x->rx_stream);
+
+	mutex_unlock(&dg00x->mutex);
+
+	return err;
+}
+
+static int pcm_capture_trigger(struct snd_pcm_substream *substream, int cmd)
+{
+	struct snd_dg00x *dg00x = substream->private_data;
+
+	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_START:
+		amdtp_stream_pcm_trigger(&dg00x->tx_stream, substream);
+		break;
+	case SNDRV_PCM_TRIGGER_STOP:
+		amdtp_stream_pcm_trigger(&dg00x->tx_stream, NULL);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+static int pcm_playback_trigger(struct snd_pcm_substream *substream, int cmd)
+{
+	struct snd_dg00x *dg00x = substream->private_data;
+
+	switch (cmd) {
+	case SNDRV_PCM_TRIGGER_START:
+		amdtp_stream_pcm_trigger(&dg00x->rx_stream, substream);
+		break;
+	case SNDRV_PCM_TRIGGER_STOP:
+		amdtp_stream_pcm_trigger(&dg00x->rx_stream, NULL);
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static snd_pcm_uframes_t pcm_capture_pointer(struct snd_pcm_substream *sbstrm)
+{
+	struct snd_dg00x *dg00x = sbstrm->private_data;
+
+	return amdtp_stream_pcm_pointer(&dg00x->tx_stream);
+}
+static snd_pcm_uframes_t pcm_playback_pointer(struct snd_pcm_substream *sbstrm)
+{
+	struct snd_dg00x *dg00x = sbstrm->private_data;
+
+	return amdtp_stream_pcm_pointer(&dg00x->rx_stream);
+}
+
+static struct snd_pcm_ops pcm_capture_ops = {
+	.open		= pcm_open,
+	.close		= pcm_close,
+	.ioctl		= snd_pcm_lib_ioctl,
+	.hw_params	= pcm_capture_hw_params,
+	.hw_free	= pcm_capture_hw_free,
+	.prepare	= pcm_capture_prepare,
+	.trigger	= pcm_capture_trigger,
+	.pointer	= pcm_capture_pointer,
+	.page		= snd_pcm_lib_get_vmalloc_page,
+};
+static struct snd_pcm_ops pcm_playback_ops = {
+	.open		= pcm_open,
+	.close		= pcm_close,
+	.ioctl		= snd_pcm_lib_ioctl,
+	.hw_params	= pcm_playback_hw_params,
+	.hw_free	= pcm_playback_hw_free,
+	.prepare	= pcm_playback_prepare,
+	.trigger	= pcm_playback_trigger,
+	.pointer	= pcm_playback_pointer,
+	.page		= snd_pcm_lib_get_vmalloc_page,
+	.mmap		= snd_pcm_lib_mmap_vmalloc,
+};
+
+int snd_dg00x_create_pcm_devices(struct snd_dg00x *dg00x)
+{
+	struct snd_pcm *pcm;
+	int err;
+
+	err = snd_pcm_new(dg00x->card, dg00x->card->driver, 0, 1, 1, &pcm);
+	if (err < 0)
+		return err;
+
+	pcm->private_data = dg00x;
+	snprintf(pcm->name, sizeof(pcm->name),
+		 "%s PCM", dg00x->card->shortname);
+	snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_PLAYBACK, &pcm_playback_ops);
+	snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_CAPTURE, &pcm_capture_ops);
+
+	return 0;
+}
+
diff --git a/sound/firewire/digi00x/digi00x.c b/sound/firewire/digi00x/digi00x.c
index d9c9e14..493437a 100644
--- a/sound/firewire/digi00x/digi00x.c
+++ b/sound/firewire/digi00x/digi00x.c
@@ -86,6 +86,10 @@ static int snd_dg00x_probe(struct fw_unit *unit,
 
 	snd_dg00x_proc_init(dg00x);
 
+	err = snd_dg00x_create_pcm_devices(dg00x);
+	if (err < 0)
+		goto error;
+
 	err = snd_card_register(card);
 	if (err < 0)
 		goto error;
diff --git a/sound/firewire/digi00x/digi00x.h b/sound/firewire/digi00x/digi00x.h
index dde039f..754486a 100644
--- a/sound/firewire/digi00x/digi00x.h
+++ b/sound/firewire/digi00x/digi00x.h
@@ -21,6 +21,8 @@
 #include <sound/core.h>
 #include <sound/initval.h>
 #include <sound/info.h>
+#include <sound/pcm.h>
+#include <sound/pcm_params.h>
 
 #include "../lib.h"
 #include "../packets-buffer.h"
@@ -84,4 +86,6 @@ void snd_dg00x_stream_update_duplex(struct snd_dg00x *dg00x);
 void snd_dg00x_stream_destroy_duplex(struct snd_dg00x *dg00x);
 
 void snd_dg00x_proc_init(struct snd_dg00x *dg00x);
+
+int snd_dg00x_create_pcm_devices(struct snd_dg00x *dg00x);
 #endif
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 05/11] ALSA: digi00x: add MIDI functionality
  2015-03-15 16:00 [RFC][PATCH 00/11] digi00x: new driver for Digidesign 002/003 family Takashi Sakamoto
                   ` (3 preceding siblings ...)
  2015-03-15 16:01 ` [PATCH 04/11] ALSA: digi00x: add PCM functionality Takashi Sakamoto
@ 2015-03-15 16:01 ` Takashi Sakamoto
  2015-03-15 16:01 ` [PATCH 06/11] ALSA: digi00x: add hwdep interface Takashi Sakamoto
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 40+ messages in thread
From: Takashi Sakamoto @ 2015-03-15 16:01 UTC (permalink / raw)
  To: clemens, tiwai, robin, damien; +Cc: alsa-devel, ffado-devel

Digi 002/003 uses AM824 format data to transfer MIDI messages. It's a
first data channel in each data block of CIP packet.

This commit adds MIDI functionality to transfer/receive MIDI messages
in the first data channel, with AMDTP functionality ALSA firewire stack
gives. But the device handles these MIDI message in disorder because
Digi 002/003 are not conformant to MMA/AMEI RP-027 or IEC 61883-6:2005.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/digi00x/Makefile         |   3 +-
 sound/firewire/digi00x/digi00x-midi.c   | 149 ++++++++++++++++++++++++++++++++
 sound/firewire/digi00x/digi00x-stream.c |   3 +
 sound/firewire/digi00x/digi00x.c        |   4 +
 sound/firewire/digi00x/digi00x.h        |   3 +
 5 files changed, 161 insertions(+), 1 deletion(-)
 create mode 100644 sound/firewire/digi00x/digi00x-midi.c

diff --git a/sound/firewire/digi00x/Makefile b/sound/firewire/digi00x/Makefile
index f2ed472..8ae72b9 100644
--- a/sound/firewire/digi00x/Makefile
+++ b/sound/firewire/digi00x/Makefile
@@ -1,2 +1,3 @@
-snd-digi00x-objs := digi00x.o digi00x-stream.o digi00x-proc.o digi00x-pcm.o
+snd-digi00x-objs := digi00x.o digi00x-stream.o digi00x-proc.o digi00x-pcm.o \
+		    digi00x-midi.o
 obj-m += snd-digi00x.o
diff --git a/sound/firewire/digi00x/digi00x-midi.c b/sound/firewire/digi00x/digi00x-midi.c
new file mode 100644
index 0000000..9936c40
--- /dev/null
+++ b/sound/firewire/digi00x/digi00x-midi.c
@@ -0,0 +1,149 @@
+/*
+ * digi00x-midi.h - a part of driver for Digidesign Digi 002/003 family
+ *
+ * Copyright (c) 2014-2015 Takashi Sakamoto
+ *
+ * Licensed under the terms of the GNU General Public License, version 2.
+ */
+
+#include "digi00x.h"
+
+static int midi_capture_open(struct snd_rawmidi_substream *substream)
+{
+	struct snd_dg00x *dg00x = substream->rmidi->private_data;
+	int err;
+
+	mutex_lock(&dg00x->mutex);
+	dg00x->capture_substreams++;
+	err = snd_dg00x_stream_start_duplex(dg00x, 0);
+	mutex_unlock(&dg00x->mutex);
+
+	return err;
+}
+
+static int midi_playback_open(struct snd_rawmidi_substream *substream)
+{
+	struct snd_dg00x *dg00x = substream->rmidi->private_data;
+	int err;
+
+	mutex_lock(&dg00x->mutex);
+	dg00x->playback_substreams++;
+	err = snd_dg00x_stream_start_duplex(dg00x, 0);
+	mutex_unlock(&dg00x->mutex);
+
+	return err;
+}
+
+static int midi_capture_close(struct snd_rawmidi_substream *substream)
+{
+	struct snd_dg00x *dg00x = substream->rmidi->private_data;
+
+	mutex_lock(&dg00x->mutex);
+	dg00x->capture_substreams--;
+	snd_dg00x_stream_stop_duplex(dg00x);
+	mutex_unlock(&dg00x->mutex);
+
+	return 0;
+}
+
+static int midi_playback_close(struct snd_rawmidi_substream *substream)
+{
+	struct snd_dg00x *dg00x = substream->rmidi->private_data;
+
+	mutex_lock(&dg00x->mutex);
+	dg00x->playback_substreams--;
+	snd_dg00x_stream_stop_duplex(dg00x);
+	mutex_unlock(&dg00x->mutex);
+
+	return 0;
+}
+
+static void midi_capture_trigger(struct snd_rawmidi_substream *substrm, int up)
+{
+	struct snd_dg00x *dg00x = substrm->rmidi->private_data;
+	unsigned long flags;
+
+	spin_lock_irqsave(&dg00x->lock, flags);
+
+	if (up)
+		amdtp_stream_midi_trigger(&dg00x->tx_stream,
+					  substrm->number, substrm);
+	else
+		amdtp_stream_midi_trigger(&dg00x->tx_stream,
+					  substrm->number, NULL);
+
+	spin_unlock_irqrestore(&dg00x->lock, flags);
+}
+
+static void midi_playback_trigger(struct snd_rawmidi_substream *substrm, int up)
+{
+	struct snd_dg00x *dg00x = substrm->rmidi->private_data;
+	unsigned long flags;
+
+	spin_lock_irqsave(&dg00x->lock, flags);
+
+	if (up)
+		amdtp_stream_midi_trigger(&dg00x->rx_stream,
+					  substrm->number, substrm);
+	else
+		amdtp_stream_midi_trigger(&dg00x->rx_stream,
+					  substrm->number, NULL);
+
+	spin_unlock_irqrestore(&dg00x->lock, flags);
+}
+
+static struct snd_rawmidi_ops midi_capture_ops = {
+	.open		= midi_capture_open,
+	.close		= midi_capture_close,
+	.trigger	= midi_capture_trigger,
+};
+
+static struct snd_rawmidi_ops midi_playback_ops = {
+	.open		= midi_playback_open,
+	.close		= midi_playback_close,
+	.trigger	= midi_playback_trigger,
+};
+
+static void set_midi_substream_names(struct snd_dg00x *dg00x,
+				     struct snd_rawmidi_str *str)
+{
+	struct snd_rawmidi_substream *subs;
+
+	list_for_each_entry(subs, &str->substreams, list) {
+		snprintf(subs->name, sizeof(subs->name),
+			 "%s MIDI %d",
+			 dg00x->card->shortname, subs->number + 1);
+	}
+}
+
+int snd_dg00x_create_midi_devices(struct snd_dg00x *dg00x)
+{
+	struct snd_rawmidi *rmidi;
+	struct snd_rawmidi_str *str;
+	int err;
+
+	err = snd_rawmidi_new(dg00x->card, dg00x->card->driver, 0,
+			      1, 2, &rmidi);
+	if (err < 0)
+		return err;
+
+	snprintf(rmidi->name, sizeof(rmidi->name),
+		 "%s MIDI", dg00x->card->shortname);
+	rmidi->private_data = dg00x;
+
+	rmidi->info_flags |= SNDRV_RAWMIDI_INFO_INPUT;
+	snd_rawmidi_set_ops(rmidi, SNDRV_RAWMIDI_STREAM_INPUT,
+			    &midi_capture_ops);
+	str = &rmidi->streams[SNDRV_RAWMIDI_STREAM_INPUT];
+	set_midi_substream_names(dg00x, str);
+
+	rmidi->info_flags |= SNDRV_RAWMIDI_INFO_OUTPUT;
+	snd_rawmidi_set_ops(rmidi, SNDRV_RAWMIDI_STREAM_OUTPUT,
+			    &midi_playback_ops);
+	str = &rmidi->streams[SNDRV_RAWMIDI_STREAM_OUTPUT];
+	set_midi_substream_names(dg00x, str);
+
+	rmidi->info_flags |= SNDRV_RAWMIDI_INFO_DUPLEX;
+
+	return 0;
+}
diff --git a/sound/firewire/digi00x/digi00x-stream.c b/sound/firewire/digi00x/digi00x-stream.c
index 75f67b5..26b6885 100644
--- a/sound/firewire/digi00x/digi00x-stream.c
+++ b/sound/firewire/digi00x/digi00x-stream.c
@@ -260,6 +260,9 @@ int snd_dg00x_stream_start_duplex(struct snd_dg00x *dg00x, unsigned int rate)
 	err = snd_dg00x_stream_get_rate(dg00x, &curr_rate);
 	if (err < 0)
 		goto error;
+	/* For MIDI substreams. */
+	if (rate == 0)
+		rate = curr_rate;
 	if ((curr_rate != rate) |
 	    amdtp_streaming_error(&dg00x->tx_stream) |
 	    amdtp_streaming_error(&dg00x->rx_stream)) {
diff --git a/sound/firewire/digi00x/digi00x.c b/sound/firewire/digi00x/digi00x.c
index 493437a..5b678ec 100644
--- a/sound/firewire/digi00x/digi00x.c
+++ b/sound/firewire/digi00x/digi00x.c
@@ -90,6 +90,10 @@ static int snd_dg00x_probe(struct fw_unit *unit,
 	if (err < 0)
 		goto error;
 
+	err = snd_dg00x_create_midi_devices(dg00x);
+	if (err < 0)
+		goto error;
+
 	err = snd_card_register(card);
 	if (err < 0)
 		goto error;
diff --git a/sound/firewire/digi00x/digi00x.h b/sound/firewire/digi00x/digi00x.h
index 754486a..b41a418 100644
--- a/sound/firewire/digi00x/digi00x.h
+++ b/sound/firewire/digi00x/digi00x.h
@@ -23,6 +23,7 @@
 #include <sound/info.h>
 #include <sound/pcm.h>
 #include <sound/pcm_params.h>
+#include <sound/rawmidi.h>
 
 #include "../lib.h"
 #include "../packets-buffer.h"
@@ -88,4 +89,6 @@ void snd_dg00x_stream_destroy_duplex(struct snd_dg00x *dg00x);
 void snd_dg00x_proc_init(struct snd_dg00x *dg00x);
 
 int snd_dg00x_create_pcm_devices(struct snd_dg00x *dg00x);
+
+int snd_dg00x_create_midi_devices(struct snd_dg00x *dg00x);
 #endif
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 06/11] ALSA: digi00x: add hwdep interface
  2015-03-15 16:00 [RFC][PATCH 00/11] digi00x: new driver for Digidesign 002/003 family Takashi Sakamoto
                   ` (4 preceding siblings ...)
  2015-03-15 16:01 ` [PATCH 05/11] ALSA: digi00x: add MIDI functionality Takashi Sakamoto
@ 2015-03-15 16:01 ` Takashi Sakamoto
  2015-03-15 16:01 ` [PATCH 07/11] ALSA: digi00x: support unknown asynchronous message Takashi Sakamoto
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 40+ messages in thread
From: Takashi Sakamoto @ 2015-03-15 16:01 UTC (permalink / raw)
  To: clemens, tiwai, robin, damien; +Cc: alsa-devel, ffado-devel

This commit adds hwdep interface so as the other firewire sound devices
has.

This interface is designed for mixer/control application. By using this
interface, an application can get information about firewire node, can
lock/unlock kernel streaming and can get notification at starting/stopping
kernel streaming.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 include/uapi/sound/asound.h             |   3 +-
 include/uapi/sound/firewire.h           |   1 +
 sound/firewire/Kconfig                  |   1 +
 sound/firewire/digi00x/Makefile         |   2 +-
 sound/firewire/digi00x/digi00x-hwdep.c  | 192 ++++++++++++++++++++++++++++++++
 sound/firewire/digi00x/digi00x-midi.c   |  14 +++
 sound/firewire/digi00x/digi00x-pcm.c    |  19 +++-
 sound/firewire/digi00x/digi00x-stream.c |  39 +++++++
 sound/firewire/digi00x/digi00x.c        |   5 +
 sound/firewire/digi00x/digi00x.h        |  13 +++
 10 files changed, 283 insertions(+), 6 deletions(-)
 create mode 100644 sound/firewire/digi00x/digi00x-hwdep.c

diff --git a/include/uapi/sound/asound.h b/include/uapi/sound/asound.h
index 46145a5..cc24d0e 100644
--- a/include/uapi/sound/asound.h
+++ b/include/uapi/sound/asound.h
@@ -100,9 +100,10 @@ enum {
 	SNDRV_HWDEP_IFACE_FW_FIREWORKS,	/* Echo Audio Fireworks based device */
 	SNDRV_HWDEP_IFACE_FW_BEBOB,	/* BridgeCo BeBoB based device */
 	SNDRV_HWDEP_IFACE_FW_OXFW,	/* Oxford OXFW970/971 based device */
+	SNDRV_HWDEP_IFACE_FW_DIGI00X,	/* Digidesign 002/003 family */
 
 	/* Don't forget to change the following: */
-	SNDRV_HWDEP_IFACE_LAST = SNDRV_HWDEP_IFACE_FW_OXFW
+	SNDRV_HWDEP_IFACE_LAST = SNDRV_HWDEP_IFACE_FW_DIGI00X
 };
 
 struct snd_hwdep_info {
diff --git a/include/uapi/sound/firewire.h b/include/uapi/sound/firewire.h
index 49122df..f67d228 100644
--- a/include/uapi/sound/firewire.h
+++ b/include/uapi/sound/firewire.h
@@ -56,6 +56,7 @@ union snd_firewire_event {
 #define SNDRV_FIREWIRE_TYPE_FIREWORKS	2
 #define SNDRV_FIREWIRE_TYPE_BEBOB	3
 #define SNDRV_FIREWIRE_TYPE_OXFW	4
+#define SNDRV_FIREWIRE_TYPE_DIGI00X	5
 /* RME, MOTU, ... */
 
 struct snd_firewire_get_info {
diff --git a/sound/firewire/Kconfig b/sound/firewire/Kconfig
index 6b9b0a1..804502b 100644
--- a/sound/firewire/Kconfig
+++ b/sound/firewire/Kconfig
@@ -121,6 +121,7 @@ config SND_BEBOB
 config SND_DIGI00X
 	tristate "Digidesign 002/003 family support"
 	select SND_FIREWIRE_LIB
+	select SND_HWDEP
 	help
 	 Say Y here to include support for Digidesign 002/003 family.
 
diff --git a/sound/firewire/digi00x/Makefile b/sound/firewire/digi00x/Makefile
index 8ae72b9..deb3683 100644
--- a/sound/firewire/digi00x/Makefile
+++ b/sound/firewire/digi00x/Makefile
@@ -1,3 +1,3 @@
 snd-digi00x-objs := digi00x.o digi00x-stream.o digi00x-proc.o digi00x-pcm.o \
-		    digi00x-midi.o
+		    digi00x-midi.o digi00x-hwdep.o
 obj-m += snd-digi00x.o
diff --git a/sound/firewire/digi00x/digi00x-hwdep.c b/sound/firewire/digi00x/digi00x-hwdep.c
new file mode 100644
index 0000000..d629e41
--- /dev/null
+++ b/sound/firewire/digi00x/digi00x-hwdep.c
@@ -0,0 +1,192 @@
+/*
+ * digi00x-hwdep.c - a part of driver for Digidesign Digi 002/003 family
+ *
+ * Copyright (c) 2014-2015 Takashi Sakamoto
+ *
+ * Licensed under the terms of the GNU General Public License, version 2.
+ */
+
+/*
+ * This codes give three functionality.
+ *
+ * 1.get firewire node information
+ * 2.get notification about starting/stopping stream
+ * 3.lock/unlock stream
+ */
+
+#include "digi00x.h"
+
+static long hwdep_read(struct snd_hwdep *hwdep, char __user *buf,  long count,
+		       loff_t *offset)
+{
+	struct snd_dg00x *dg00x = hwdep->private_data;
+	DEFINE_WAIT(wait);
+	union snd_firewire_event event;
+
+	spin_lock_irq(&dg00x->lock);
+
+	while (!dg00x->dev_lock_changed) {
+		prepare_to_wait(&dg00x->hwdep_wait, &wait, TASK_INTERRUPTIBLE);
+		spin_unlock_irq(&dg00x->lock);
+		schedule();
+		finish_wait(&dg00x->hwdep_wait, &wait);
+		if (signal_pending(current))
+			return -ERESTARTSYS;
+		spin_lock_irq(&dg00x->lock);
+	}
+
+	memset(&event, 0, sizeof(event));
+	if (dg00x->dev_lock_changed) {
+		event.lock_status.type = SNDRV_FIREWIRE_EVENT_LOCK_STATUS;
+		event.lock_status.status = (dg00x->dev_lock_count > 0);
+		dg00x->dev_lock_changed = false;
+
+		count = min_t(long, count, sizeof(event.lock_status));
+	}
+
+	spin_unlock_irq(&dg00x->lock);
+
+	if (copy_to_user(buf, &event, count))
+		return -EFAULT;
+
+	return count;
+}
+
+static unsigned int hwdep_poll(struct snd_hwdep *hwdep, struct file *file,
+			       poll_table *wait)
+{
+	struct snd_dg00x *dg00x = hwdep->private_data;
+	unsigned int events;
+
+	poll_wait(file, &dg00x->hwdep_wait, wait);
+
+	spin_lock_irq(&dg00x->lock);
+	if (dg00x->dev_lock_changed)
+		events = POLLIN | POLLRDNORM;
+	else
+		events = 0;
+	spin_unlock_irq(&dg00x->lock);
+
+	return events;
+}
+
+static int hwdep_get_info(struct snd_dg00x *dg00x, void __user *arg)
+{
+	struct fw_device *dev = fw_parent_device(dg00x->unit);
+	struct snd_firewire_get_info info;
+
+	memset(&info, 0, sizeof(info));
+	info.type = SNDRV_FIREWIRE_TYPE_DIGI00X;
+	info.card = dev->card->index;
+	*(__be32 *)&info.guid[0] = cpu_to_be32(dev->config_rom[3]);
+	*(__be32 *)&info.guid[4] = cpu_to_be32(dev->config_rom[4]);
+	strlcpy(info.device_name, dev_name(&dev->device),
+		sizeof(info.device_name));
+
+	if (copy_to_user(arg, &info, sizeof(info)))
+		return -EFAULT;
+
+	return 0;
+}
+
+static int hwdep_lock(struct snd_dg00x *dg00x)
+{
+	int err;
+
+	spin_lock_irq(&dg00x->lock);
+
+	if (dg00x->dev_lock_count == 0) {
+		dg00x->dev_lock_count = -1;
+		err = 0;
+	} else {
+		err = -EBUSY;
+	}
+
+	spin_unlock_irq(&dg00x->lock);
+
+	return err;
+}
+
+static int hwdep_unlock(struct snd_dg00x *dg00x)
+{
+	int err;
+
+	spin_lock_irq(&dg00x->lock);
+
+	if (dg00x->dev_lock_count == -1) {
+		dg00x->dev_lock_count = 0;
+		err = 0;
+	} else {
+		err = -EBADFD;
+	}
+
+	spin_unlock_irq(&dg00x->lock);
+
+	return err;
+}
+
+static int hwdep_release(struct snd_hwdep *hwdep, struct file *file)
+{
+	struct snd_dg00x *dg00x = hwdep->private_data;
+
+	spin_lock_irq(&dg00x->lock);
+	if (dg00x->dev_lock_count == -1)
+		dg00x->dev_lock_count = 0;
+	spin_unlock_irq(&dg00x->lock);
+
+	return 0;
+}
+
+static int hwdep_ioctl(struct snd_hwdep *hwdep, struct file *file,
+	    unsigned int cmd, unsigned long arg)
+{
+	struct snd_dg00x *dg00x = hwdep->private_data;
+
+	switch (cmd) {
+	case SNDRV_FIREWIRE_IOCTL_GET_INFO:
+		return hwdep_get_info(dg00x, (void __user *)arg);
+	case SNDRV_FIREWIRE_IOCTL_LOCK:
+		return hwdep_lock(dg00x);
+	case SNDRV_FIREWIRE_IOCTL_UNLOCK:
+		return hwdep_unlock(dg00x);
+	default:
+		return -ENOIOCTLCMD;
+	}
+}
+
+#ifdef CONFIG_COMPAT
+static int hwdep_compat_ioctl(struct snd_hwdep *hwdep, struct file *file,
+			      unsigned int cmd, unsigned long arg)
+{
+	return hwdep_ioctl(hwdep, file, cmd,
+			   (unsigned long)compat_ptr(arg));
+}
+#else
+#define hwdep_compat_ioctl NULL
+#endif
+
+static const struct snd_hwdep_ops hwdep_ops = {
+	.read		= hwdep_read,
+	.release	= hwdep_release,
+	.poll		= hwdep_poll,
+	.ioctl		= hwdep_ioctl,
+	.ioctl_compat	= hwdep_compat_ioctl,
+};
+
+int snd_dg00x_create_hwdep_device(struct snd_dg00x *dg00x)
+{
+	struct snd_hwdep *hwdep;
+	int err;
+
+	err = snd_hwdep_new(dg00x->card, "Digi00x", 0, &hwdep);
+	if (err < 0)
+		return err;
+
+	strcpy(hwdep->name, "Digi00x");
+	hwdep->iface = SNDRV_HWDEP_IFACE_FW_DIGI00X;
+	hwdep->ops = hwdep_ops;
+	hwdep->private_data = dg00x;
+	hwdep->exclusive = true;
+
+	return err;
+}
diff --git a/sound/firewire/digi00x/digi00x-midi.c b/sound/firewire/digi00x/digi00x-midi.c
index 9936c40..460f8eb 100644
--- a/sound/firewire/digi00x/digi00x-midi.c
+++ b/sound/firewire/digi00x/digi00x-midi.c
@@ -13,10 +13,16 @@ static int midi_capture_open(struct snd_rawmidi_substream *substream)
 	struct snd_dg00x *dg00x = substream->rmidi->private_data;
 	int err;
 
+	err = snd_dg00x_stream_lock_try(dg00x);
+	if (err < 0)
+		return err;
+
 	mutex_lock(&dg00x->mutex);
 	dg00x->capture_substreams++;
 	err = snd_dg00x_stream_start_duplex(dg00x, 0);
 	mutex_unlock(&dg00x->mutex);
+	if (err < 0)
+		snd_dg00x_stream_lock_release(dg00x);
 
 	return err;
 }
@@ -26,10 +32,16 @@ static int midi_playback_open(struct snd_rawmidi_substream *substream)
 	struct snd_dg00x *dg00x = substream->rmidi->private_data;
 	int err;
 
+	err = snd_dg00x_stream_lock_try(dg00x);
+	if (err < 0)
+		return err;
+
 	mutex_lock(&dg00x->mutex);
 	dg00x->playback_substreams++;
 	err = snd_dg00x_stream_start_duplex(dg00x, 0);
 	mutex_unlock(&dg00x->mutex);
+	if (err < 0)
+		snd_dg00x_stream_lock_release(dg00x);
 
 	return err;
 }
@@ -43,6 +55,7 @@ static int midi_capture_close(struct snd_rawmidi_substream *substream)
 	snd_dg00x_stream_stop_duplex(dg00x);
 	mutex_unlock(&dg00x->mutex);
 
+	snd_dg00x_stream_lock_release(dg00x);
 	return 0;
 }
 
@@ -55,6 +68,7 @@ static int midi_playback_close(struct snd_rawmidi_substream *substream)
 	snd_dg00x_stream_stop_duplex(dg00x);
 	mutex_unlock(&dg00x->mutex);
 
+	snd_dg00x_stream_lock_release(dg00x);
 	return 0;
 }
 
diff --git a/sound/firewire/digi00x/digi00x-pcm.c b/sound/firewire/digi00x/digi00x-pcm.c
index 4522b38..52c17d5 100644
--- a/sound/firewire/digi00x/digi00x-pcm.c
+++ b/sound/firewire/digi00x/digi00x-pcm.c
@@ -115,26 +115,37 @@ static int pcm_open(struct snd_pcm_substream *substream)
 	unsigned int rate;
 	int err;
 
+	err = snd_dg00x_stream_lock_try(dg00x);
+	if (err < 0)
+		goto end;
+
 	err = pcm_init_hw_params(dg00x, substream);
 	if (err < 0)
-		return err;
+		goto err_locked;
 
 	err = snd_dg00x_stream_get_clock(dg00x, &clock);
 	if (clock != SND_DG00X_CLOCK_INTERNAL) {
 		err = snd_dg00x_stream_get_rate(dg00x, &rate);
 		if (err < 0)
-			return err;
+			goto err_locked;
 		substream->runtime->hw.rate_min = rate;
 		substream->runtime->hw.rate_max = rate;
 	}
 
 	snd_pcm_set_sync(substream);
-
-	return 0;
+end:
+	return err;
+err_locked:
+	snd_dg00x_stream_lock_release(dg00x);
+	return err;
 }
 
 static int pcm_close(struct snd_pcm_substream *substream)
 {
+	struct snd_dg00x *dg00x = substream->private_data;
+
+	snd_dg00x_stream_lock_release(dg00x);
+
 	return 0;
 }
 
diff --git a/sound/firewire/digi00x/digi00x-stream.c b/sound/firewire/digi00x/digi00x-stream.c
index 26b6885..92eb86d 100644
--- a/sound/firewire/digi00x/digi00x-stream.c
+++ b/sound/firewire/digi00x/digi00x-stream.c
@@ -352,3 +352,42 @@ void snd_dg00x_stream_update_duplex(struct snd_dg00x *dg00x)
 	amdtp_stream_update(&dg00x->tx_stream);
 	amdtp_stream_update(&dg00x->rx_stream);
 }
+
+void snd_dg00x_stream_lock_changed(struct snd_dg00x *dg00x)
+{
+	dg00x->dev_lock_changed = true;
+	wake_up(&dg00x->hwdep_wait);
+}
+
+int snd_dg00x_stream_lock_try(struct snd_dg00x *dg00x)
+{
+	int err;
+
+	spin_lock_irq(&dg00x->lock);
+
+	/* user land lock this */
+	if (dg00x->dev_lock_count < 0) {
+		err = -EBUSY;
+		goto end;
+	}
+
+	/* this is the first time */
+	if (dg00x->dev_lock_count++ == 0)
+		snd_dg00x_stream_lock_changed(dg00x);
+	err = 0;
+end:
+	spin_unlock_irq(&dg00x->lock);
+	return err;
+}
+
+void snd_dg00x_stream_lock_release(struct snd_dg00x *dg00x)
+{
+	spin_lock_irq(&dg00x->lock);
+
+	if (WARN_ON(dg00x->dev_lock_count <= 0))
+		goto end;
+	if (--dg00x->dev_lock_count == 0)
+		snd_dg00x_stream_lock_changed(dg00x);
+end:
+	spin_unlock_irq(&dg00x->lock);
+}
diff --git a/sound/firewire/digi00x/digi00x.c b/sound/firewire/digi00x/digi00x.c
index 5b678ec..c170cbb 100644
--- a/sound/firewire/digi00x/digi00x.c
+++ b/sound/firewire/digi00x/digi00x.c
@@ -75,6 +75,7 @@ static int snd_dg00x_probe(struct fw_unit *unit,
 
 	mutex_init(&dg00x->mutex);
 	spin_lock_init(&dg00x->lock);
+	init_waitqueue_head(&dg00x->hwdep_wait);
 
 	err = name_card(dg00x);
 	if (err < 0)
@@ -94,6 +95,10 @@ static int snd_dg00x_probe(struct fw_unit *unit,
 	if (err < 0)
 		goto error;
 
+	err = snd_dg00x_create_hwdep_device(dg00x);
+	if (err < 0)
+		goto error;
+
 	err = snd_card_register(card);
 	if (err < 0)
 		goto error;
diff --git a/sound/firewire/digi00x/digi00x.h b/sound/firewire/digi00x/digi00x.h
index b41a418..7503d31 100644
--- a/sound/firewire/digi00x/digi00x.h
+++ b/sound/firewire/digi00x/digi00x.h
@@ -24,6 +24,8 @@
 #include <sound/pcm.h>
 #include <sound/pcm_params.h>
 #include <sound/rawmidi.h>
+#include <sound/hwdep.h>
+#include <sound/firewire.h>
 
 #include "../lib.h"
 #include "../packets-buffer.h"
@@ -46,6 +48,11 @@ struct snd_dg00x {
 
 	unsigned int playback_substreams;
 	unsigned int capture_substreams;
+
+	/* for uapi */
+	int dev_lock_count;
+	bool dev_lock_changed;
+	wait_queue_head_t hwdep_wait;
 };
 
 /* values for SND_DG00X_ADDR_OFFSET_RATE */
@@ -86,9 +93,15 @@ void snd_dg00x_stream_stop_duplex(struct snd_dg00x *dg00x);
 void snd_dg00x_stream_update_duplex(struct snd_dg00x *dg00x);
 void snd_dg00x_stream_destroy_duplex(struct snd_dg00x *dg00x);
 
+void snd_dg00x_stream_lock_changed(struct snd_dg00x *dg00x);
+int snd_dg00x_stream_lock_try(struct snd_dg00x *dg00x);
+void snd_dg00x_stream_lock_release(struct snd_dg00x *dg00x);
+
 void snd_dg00x_proc_init(struct snd_dg00x *dg00x);
 
 int snd_dg00x_create_pcm_devices(struct snd_dg00x *dg00x);
 
 int snd_dg00x_create_midi_devices(struct snd_dg00x *dg00x);
+
+int snd_dg00x_create_hwdep_device(struct snd_dg00x *dg00x);
 #endif
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 07/11] ALSA: digi00x: support unknown asynchronous message
  2015-03-15 16:00 [RFC][PATCH 00/11] digi00x: new driver for Digidesign 002/003 family Takashi Sakamoto
                   ` (5 preceding siblings ...)
  2015-03-15 16:01 ` [PATCH 06/11] ALSA: digi00x: add hwdep interface Takashi Sakamoto
@ 2015-03-15 16:01 ` Takashi Sakamoto
  2015-03-15 16:01 ` [PATCH 08/11] ALSA: digi00x: support MIDI ports for device control Takashi Sakamoto
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 40+ messages in thread
From: Takashi Sakamoto @ 2015-03-15 16:01 UTC (permalink / raw)
  To: clemens, tiwai, robin, damien; +Cc: alsa-devel, ffado-devel

Digi 002/003 family use asynchronous transaction for messaging.
The address to receive this message is stored on a certain register.

This commit allocates a certain range of address on OHCI 1394 host
controller, to handle this notification. Currently, the purpose of
this message is unknown.

Usually, these devices notify 0x00007051. But there're cases that
0x00007052/0200007058. In these cases, the device sounds quite a
short gap.

I guess that this notification is related to clock synchronization.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/digi00x/Makefile           |   2 +-
 sound/firewire/digi00x/digi00x-protocol.c | 122 ++++++++++++++++++++++++++++++
 sound/firewire/digi00x/digi00x.c          |  12 +++
 sound/firewire/digi00x/digi00x.h          |   5 ++
 4 files changed, 140 insertions(+), 1 deletion(-)
 create mode 100644 sound/firewire/digi00x/digi00x-protocol.c

diff --git a/sound/firewire/digi00x/Makefile b/sound/firewire/digi00x/Makefile
index deb3683..e42b5cc 100644
--- a/sound/firewire/digi00x/Makefile
+++ b/sound/firewire/digi00x/Makefile
@@ -1,3 +1,3 @@
 snd-digi00x-objs := digi00x.o digi00x-stream.o digi00x-proc.o digi00x-pcm.o \
-		    digi00x-midi.o digi00x-hwdep.o
+		    digi00x-midi.o digi00x-hwdep.o digi00x-protocol.o
 obj-m += snd-digi00x.o
diff --git a/sound/firewire/digi00x/digi00x-protocol.c b/sound/firewire/digi00x/digi00x-protocol.c
new file mode 100644
index 0000000..b19708d
--- /dev/null
+++ b/sound/firewire/digi00x/digi00x-protocol.c
@@ -0,0 +1,122 @@
+/*
+ * digi00x-protocol.c - a part of driver for Digidesign Digi 002/003 family
+ *
+ * Copyright (c) 2014-2015 Takashi Sakamoto
+ *
+ * Licensed under the terms of the GNU General Public License, version 2.
+ */
+
+#include "digi00x.h"
+
+static struct snd_dg00x *instances[SNDRV_CARDS];
+static DEFINE_SPINLOCK(instances_lock);
+
+static void handle_unknown_message(struct snd_dg00x *dg00x,
+				   unsigned long long offset, u32 *buf)
+{
+	snd_printk(KERN_INFO"%08llx: %08x\n", offset, be32_to_cpu(*buf));
+}
+
+static void handle_message(struct fw_card *card, struct fw_request *request,
+			   int tcode, int destination, int source,
+			   int generation, unsigned long long offset,
+			   void *data, size_t length, void *callback_data)
+{
+	u32 *buf = (__be32 *)data;
+	struct fw_device *device;
+	struct snd_dg00x *dg00x;
+	unsigned int i;
+
+	spin_lock_irq(&instances_lock);
+	for (i = 0; i < SNDRV_CARDS; i++) {
+		dg00x = instances[i];
+		if (dg00x == NULL)
+			continue;
+		device = fw_parent_device(dg00x->unit);
+		if (device->card != card)
+			continue;
+		smp_rmb();	/* node id vs. generation */
+		if (device->node_id != source)
+			continue;
+		break;
+	}
+
+	if (offset == 0xffffe0000000)
+		handle_unknown_message(dg00x, offset, buf);
+
+	spin_unlock_irq(&instances_lock);
+	fw_send_response(card, request, RCODE_COMPLETE);
+}
+
+/*
+ * Use the same range of address for asynchronous messages from any devices, to
+ * save resources on host controller.
+ */
+static struct fw_address_handler async_handler;
+
+int snd_dg00x_protocol_add_instance(struct snd_dg00x *dg00x)
+{
+	struct fw_device *device = fw_parent_device(dg00x->unit);
+	__be32 data[2];
+	unsigned int i;
+	int err;
+
+	/* Unknown. 4bytes. */
+	data[0] = cpu_to_be32((device->card->node_id << 16) |
+			      (async_handler.offset >> 32));
+	data[1] = cpu_to_be32(async_handler.offset);
+	err = snd_fw_transaction(dg00x->unit, TCODE_WRITE_BLOCK_REQUEST,
+				 0xffffe0000014ull, &data, sizeof(data), 0);
+	if (err < 0)
+		return err;
+
+	spin_lock_irq(&instances_lock);
+	for (i = 0; i < SNDRV_CARDS; i++) {
+		if (instances[i] != NULL)
+			continue;
+		instances[i] = dg00x;
+		break;
+	}
+	spin_unlock_irq(&instances_lock);
+
+	return 0;
+}
+
+void snd_dg00x_protocol_remove_instance(struct snd_dg00x *dg00x)
+{
+	unsigned int i;
+
+	spin_lock_irq(&instances_lock);
+	for (i = 0; i < SNDRV_CARDS; i++) {
+		if (instances[i] != dg00x)
+			continue;
+		instances[i] = NULL;
+		break;
+	}
+	spin_unlock_irq(&instances_lock);
+}
+
+int snd_dg00x_protocol_register(void)
+{
+	static const struct fw_address_region resp_register_region = {
+		.start	= 0xffffe0000000ull,
+		.end	= 0xffffe000ffffull,
+	};
+	int err;
+
+	async_handler.length = 4;
+	async_handler.address_callback = handle_message;
+	async_handler.callback_data = NULL;
+
+	err = fw_core_add_address_handler(&async_handler,
+					  &resp_register_region);
+	if (err < 0)
+		return err;
+
+	return 0;
+}
+
+void snd_dg00x_protocol_unregister(void)
+{
+	fw_core_remove_address_handler(&async_handler);
+}
diff --git a/sound/firewire/digi00x/digi00x.c b/sound/firewire/digi00x/digi00x.c
index c170cbb..db0997d 100644
--- a/sound/firewire/digi00x/digi00x.c
+++ b/sound/firewire/digi00x/digi00x.c
@@ -48,6 +48,7 @@ static void dg00x_card_free(struct snd_card *card)
 	struct snd_dg00x *dg00x = card->private_data;
 
 	snd_dg00x_stream_destroy_duplex(dg00x);
+	snd_dg00x_protocol_remove_instance(dg00x);
 
 	fw_unit_put(dg00x->unit);
 
@@ -99,6 +100,10 @@ static int snd_dg00x_probe(struct fw_unit *unit,
 	if (err < 0)
 		goto error;
 
+	err = snd_dg00x_protocol_add_instance(dg00x);
+	if (err < 0)
+		goto error;
+
 	err = snd_card_register(card);
 	if (err < 0)
 		goto error;
@@ -155,11 +160,18 @@ static struct fw_driver dg00x_driver = {
 
 static int __init snd_dg00x_init(void)
 {
+	int err;
+
+	err = snd_dg00x_protocol_register();
+	if (err < 0)
+		return err;
+
 	return driver_register(&dg00x_driver.driver);
 }
 
 static void __exit snd_dg00x_exit(void)
 {
+	snd_dg00x_protocol_unregister();
 	driver_unregister(&dg00x_driver.driver);
 }
 
diff --git a/sound/firewire/digi00x/digi00x.h b/sound/firewire/digi00x/digi00x.h
index 7503d31..85cfb39 100644
--- a/sound/firewire/digi00x/digi00x.h
+++ b/sound/firewire/digi00x/digi00x.h
@@ -78,6 +78,11 @@ enum snd_dg00x_optical_mode {
 	SND_DG00X_OPTICAL_MODE_SPDIF,
 };
 
+int snd_dg00x_protocol_add_instance(struct snd_dg00x *dg00x);
+void snd_dg00x_protocol_remove_instance(struct snd_dg00x *dg00x);
+int snd_dg00x_protocol_register(void);
+void snd_dg00x_protocol_unregister(void);
+
 extern const unsigned int snd_dg00x_stream_rates[SND_DG00X_RATE_COUNT];
 extern const unsigned int
 snd_dg00x_stream_mbla_data_channels[SND_DG00X_RATE_COUNT];
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 08/11] ALSA: digi00x: support MIDI ports for device control
  2015-03-15 16:00 [RFC][PATCH 00/11] digi00x: new driver for Digidesign 002/003 family Takashi Sakamoto
                   ` (6 preceding siblings ...)
  2015-03-15 16:01 ` [PATCH 07/11] ALSA: digi00x: support unknown asynchronous message Takashi Sakamoto
@ 2015-03-15 16:01 ` Takashi Sakamoto
  2015-03-15 16:01 ` [PATCH 09/11] ALSA: firewire-lib: allows to implement external MIDI callback function Takashi Sakamoto
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 40+ messages in thread
From: Takashi Sakamoto @ 2015-03-15 16:01 UTC (permalink / raw)
  To: clemens, tiwai, robin, damien; +Cc: alsa-devel, ffado-devel

Digi 002/003 family uses asynchronous transactions for MIDI
device control message. The address to transfer is stored on
a certain address, while the address to receive is 0xffffe0000040.

This commit supports MIDI ports for this purpose. For capture
MIDI message, the handler of notification is extended. For playback
MIDI message, a workqueue is used because Linux FireWire subsystem
uses 'complete' to wait response event and this context should be
able to sleep.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/digi00x/digi00x-midi.c     | 68 +++++++++++++++++++------
 sound/firewire/digi00x/digi00x-protocol.c | 82 ++++++++++++++++++++++++++++++-
 sound/firewire/digi00x/digi00x.h          |  7 +++
 3 files changed, 139 insertions(+), 18 deletions(-)

diff --git a/sound/firewire/digi00x/digi00x-midi.c b/sound/firewire/digi00x/digi00x-midi.c
index 460f8eb..c4990ad 100644
--- a/sound/firewire/digi00x/digi00x-midi.c
+++ b/sound/firewire/digi00x/digi00x-midi.c
@@ -13,6 +13,10 @@ static int midi_capture_open(struct snd_rawmidi_substream *substream)
 	struct snd_dg00x *dg00x = substream->rmidi->private_data;
 	int err;
 
+	/* This port is for Asynchronous transaction. */
+	if (substream->number == 0)
+		return 0;
+
 	err = snd_dg00x_stream_lock_try(dg00x);
 	if (err < 0)
 		return err;
@@ -32,6 +36,10 @@ static int midi_playback_open(struct snd_rawmidi_substream *substream)
 	struct snd_dg00x *dg00x = substream->rmidi->private_data;
 	int err;
 
+	/* This port is for Asynchronous transaction. */
+	if (substream->number == 0)
+		return 0;
+
 	err = snd_dg00x_stream_lock_try(dg00x);
 	if (err < 0)
 		return err;
@@ -50,6 +58,9 @@ static int midi_capture_close(struct snd_rawmidi_substream *substream)
 {
 	struct snd_dg00x *dg00x = substream->rmidi->private_data;
 
+	if (substream->number == 0)
+		return 0;
+
 	mutex_lock(&dg00x->mutex);
 	dg00x->capture_substreams--;
 	snd_dg00x_stream_stop_duplex(dg00x);
@@ -63,6 +74,9 @@ static int midi_playback_close(struct snd_rawmidi_substream *substream)
 {
 	struct snd_dg00x *dg00x = substream->rmidi->private_data;
 
+	if (substream->number == 0)
+		return 0;
+
 	mutex_lock(&dg00x->mutex);
 	dg00x->playback_substreams--;
 	snd_dg00x_stream_stop_duplex(dg00x);
@@ -79,12 +93,19 @@ static void midi_capture_trigger(struct snd_rawmidi_substream *substrm, int up)
 
 	spin_lock_irqsave(&dg00x->lock, flags);
 
-	if (up)
-		amdtp_stream_midi_trigger(&dg00x->tx_stream,
-					  substrm->number, substrm);
-	else
-		amdtp_stream_midi_trigger(&dg00x->tx_stream,
-					  substrm->number, NULL);
+	if (substrm->number == 0) {
+		if (up)
+			dg00x->in_control = substrm;
+		else
+			dg00x->in_control = NULL;
+	} else {
+		if (up)
+			amdtp_stream_midi_trigger(&dg00x->tx_stream,
+						  substrm->number - 1, substrm);
+		else
+			amdtp_stream_midi_trigger(&dg00x->tx_stream,
+						  substrm->number - 1, NULL);
+	}
 
 	spin_unlock_irqrestore(&dg00x->lock, flags);
 }
@@ -96,12 +117,21 @@ static void midi_playback_trigger(struct snd_rawmidi_substream *substrm, int up)
 
 	spin_lock_irqsave(&dg00x->lock, flags);
 
-	if (up)
-		amdtp_stream_midi_trigger(&dg00x->rx_stream,
-					  substrm->number, substrm);
-	else
-		amdtp_stream_midi_trigger(&dg00x->rx_stream,
-					  substrm->number, NULL);
+	if (substrm->number == 0) {
+		if (up) {
+			dg00x->out_control = substrm;
+			snd_dg00x_protocol_queue_midi_message(dg00x);
+		} else {
+			dg00x->out_control = NULL;
+		}
+	} else {
+		if (up)
+			amdtp_stream_midi_trigger(&dg00x->rx_stream,
+						  substrm->number - 1, substrm);
+		else
+			amdtp_stream_midi_trigger(&dg00x->rx_stream,
+						  substrm->number - 1, NULL);
+	}
 
 	spin_unlock_irqrestore(&dg00x->lock, flags);
 }
@@ -124,9 +154,15 @@ static void set_midi_substream_names(struct snd_dg00x *dg00x,
 	struct snd_rawmidi_substream *subs;
 
 	list_for_each_entry(subs, &str->substreams, list) {
-		snprintf(subs->name, sizeof(subs->name),
-			 "%s MIDI %d",
-			 dg00x->card->shortname, subs->number + 1);
+		/* This port is for device control. */
+		if (subs->number == 0) {
+			snprintf(subs->name, sizeof(subs->name),
+				 "%s control", dg00x->card->shortname);
+		} else {
+			snprintf(subs->name, sizeof(subs->name),
+				 "%s MIDI %d",
+				 dg00x->card->shortname, subs->number + 1);
+		}
 	}
 }
 
@@ -137,7 +173,7 @@ int snd_dg00x_create_midi_devices(struct snd_dg00x *dg00x)
 	int err;
 
 	err = snd_rawmidi_new(dg00x->card, dg00x->card->driver, 0,
-			      1, 2, &rmidi);
+			      3, 2, &rmidi);
 	if (err < 0)
 		return err;
 
diff --git a/sound/firewire/digi00x/digi00x-protocol.c b/sound/firewire/digi00x/digi00x-protocol.c
index b19708d..3e5b3bec 100644
--- a/sound/firewire/digi00x/digi00x-protocol.c
+++ b/sound/firewire/digi00x/digi00x-protocol.c
@@ -8,6 +8,43 @@
 
 #include "digi00x.h"
 
+struct workqueue_struct *midi_wq;
+
+static void send_midi_control(struct work_struct *work)
+{
+	struct snd_dg00x *dg00x =
+			container_of(work, struct snd_dg00x, midi_control);
+	struct fw_device *device = fw_parent_device(dg00x->unit);
+
+	unsigned int len;
+	__be32 buf = 0;
+	u8 *b = (u8 *)&buf;
+
+	/* Send MIDI control. */
+	if (!dg00x->out_control)
+		return;
+
+	do {
+		len = snd_rawmidi_transmit(dg00x->out_control, b + 1, 2);
+		if (len > 0) {
+			b[0] = 0x80;
+			b[3] = 0xc0 | len;
+
+			/* Don't check transaction status. */
+			fw_run_transaction(device->card,
+					   TCODE_WRITE_QUADLET_REQUEST,
+					   device->node_id, device->generation,
+					   device->max_speed,
+					   0xffffe0000400, &buf, sizeof(buf));
+		}
+	} while (len > 0);
+}
+
+void snd_dg00x_protocol_queue_midi_message(struct snd_dg00x *dg00x)
+{
+	queue_work(midi_wq, &dg00x->midi_control);
+}
+
 static struct snd_dg00x *instances[SNDRV_CARDS];
 static DEFINE_SPINLOCK(instances_lock);
 
@@ -17,6 +54,26 @@ static void handle_unknown_message(struct snd_dg00x *dg00x,
 	snd_printk(KERN_INFO"%08llx: %08x\n", offset, be32_to_cpu(*buf));
 }
 
+static void handle_midi_control(struct snd_dg00x *dg00x, u32 *buf,
+				unsigned int length)
+{
+	unsigned int i;
+	unsigned int len;
+	u8 *b;
+
+	if (dg00x->in_control == NULL)
+		return;
+
+	length /= 4;
+
+	for (i = 0; i < length; i++) {
+		b = (u8 *)&buf[i];
+		len = b[3] & 0xf;
+		if (len > 0)
+			snd_rawmidi_receive(dg00x->in_control, b + 1, len);
+	}
+}
+
 static void handle_message(struct fw_card *card, struct fw_request *request,
 			   int tcode, int destination, int source,
 			   int generation, unsigned long long offset,
@@ -43,6 +100,8 @@ static void handle_message(struct fw_card *card, struct fw_request *request,
 
 	if (offset == 0xffffe0000000)
 		handle_unknown_message(dg00x, offset, buf);
+	else if (offset == 0xffffe0000004)
+		handle_midi_control(dg00x, buf, length);
 
 	spin_unlock_irq(&instances_lock);
 	fw_send_response(card, request, RCODE_COMPLETE);
@@ -70,6 +129,15 @@ int snd_dg00x_protocol_add_instance(struct snd_dg00x *dg00x)
 	if (err < 0)
 		return err;
 
+	/* Asynchronous transactions for MIDI control message. 8 bytes. */
+	data[0] = cpu_to_be32((device->card->node_id << 16) |
+			      (async_handler.offset >> 32));
+	data[1] = cpu_to_be32(async_handler.offset + 4);
+	err = snd_fw_transaction(dg00x->unit, TCODE_WRITE_BLOCK_REQUEST,
+				 0xffffe0000008ull, &data, sizeof(data), 0);
+	if (err < 0)
+		return err;
+
 	spin_lock_irq(&instances_lock);
 	for (i = 0; i < SNDRV_CARDS; i++) {
 		if (instances[i] != NULL)
@@ -79,6 +147,8 @@ int snd_dg00x_protocol_add_instance(struct snd_dg00x *dg00x)
 	}
 	spin_unlock_irq(&instances_lock);
 
+	INIT_WORK(&dg00x->midi_control, send_midi_control);
+
 	return 0;
 }
 
@@ -104,19 +174,27 @@ int snd_dg00x_protocol_register(void)
 	};
 	int err;
 
-	async_handler.length = 4;
+	midi_wq = alloc_workqueue("snd-digi00x",
+				  WQ_SYSFS | WQ_POWER_EFFICIENT, 0);
+	if (midi_wq == NULL)
+		return -ENOMEM;
+
+	async_handler.length = 12;
 	async_handler.address_callback = handle_message;
 	async_handler.callback_data = NULL;
 
 	err = fw_core_add_address_handler(&async_handler,
 					  &resp_register_region);
-	if (err < 0)
+	if (err < 0) {
+		destroy_workqueue(midi_wq);
 		return err;
+	}
 
 	return 0;
 }
 
 void snd_dg00x_protocol_unregister(void)
 {
+	destroy_workqueue(midi_wq);
 	fw_core_remove_address_handler(&async_handler);
 }
diff --git a/sound/firewire/digi00x/digi00x.h b/sound/firewire/digi00x/digi00x.h
index 85cfb39..20b178f 100644
--- a/sound/firewire/digi00x/digi00x.h
+++ b/sound/firewire/digi00x/digi00x.h
@@ -17,6 +17,7 @@
 #include <linux/mod_devicetable.h>
 #include <linux/delay.h>
 #include <linux/slab.h>
+#include <linux/workqueue.h>
 
 #include <sound/core.h>
 #include <sound/initval.h>
@@ -53,6 +54,11 @@ struct snd_dg00x {
 	int dev_lock_count;
 	bool dev_lock_changed;
 	wait_queue_head_t hwdep_wait;
+
+	/* For asynchronous MIDI controls. */
+	struct work_struct midi_control;
+	struct snd_rawmidi_substream *in_control;
+	struct snd_rawmidi_substream *out_control;
 };
 
 /* values for SND_DG00X_ADDR_OFFSET_RATE */
@@ -78,6 +84,7 @@ enum snd_dg00x_optical_mode {
 	SND_DG00X_OPTICAL_MODE_SPDIF,
 };
 
+void snd_dg00x_protocol_queue_midi_message(struct snd_dg00x *dg00x);
 int snd_dg00x_protocol_add_instance(struct snd_dg00x *dg00x);
 void snd_dg00x_protocol_remove_instance(struct snd_dg00x *dg00x);
 int snd_dg00x_protocol_register(void);
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 09/11] ALSA: firewire-lib: allows to implement external MIDI callback function
  2015-03-15 16:00 [RFC][PATCH 00/11] digi00x: new driver for Digidesign 002/003 family Takashi Sakamoto
                   ` (7 preceding siblings ...)
  2015-03-15 16:01 ` [PATCH 08/11] ALSA: digi00x: support MIDI ports for device control Takashi Sakamoto
@ 2015-03-15 16:01 ` Takashi Sakamoto
  2015-03-15 16:01 ` [PATCH 10/11] digi00x: improve MIDI capture/playback Takashi Sakamoto
  2015-03-15 16:01 ` [PATCH 11/11] ALSA: digi00x: apply double-oh-three algorism to multiplex PCM samples Takashi Sakamoto
  10 siblings, 0 replies; 40+ messages in thread
From: Takashi Sakamoto @ 2015-03-15 16:01 UTC (permalink / raw)
  To: clemens, tiwai, robin, damien; +Cc: alsa-devel, ffado-devel

This commit is a preparation for devices unconformant to IEC 61883-1:2005
or MMA/AMEI RP-027. This commit allows each driver to implement own MIDI
callback functions.

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

diff --git a/sound/firewire/amdtp.c b/sound/firewire/amdtp.c
index e061355..2758fb8 100644
--- a/sound/firewire/amdtp.c
+++ b/sound/firewire/amdtp.c
@@ -68,6 +68,11 @@
 
 static void pcm_period_tasklet(unsigned long data);
 
+static void amdtp_fill_midi(struct amdtp_stream *s,
+			    __be32 *buffer, unsigned int frames);
+static void amdtp_pull_midi(struct amdtp_stream *s,
+			    __be32 *buffer, unsigned int frames);
+
 /**
  * amdtp_stream_init - initialize an AMDTP stream structure
  * @s: the AMDTP stream to initialize
@@ -236,7 +241,7 @@ sfc_found:
 	 * We do not know the actual MIDI FIFO size of most devices.  Just
 	 * assume two bytes, i.e., one byte can be received over the bus while
 	 * the previous one is transmitted over MIDI.
-	 * (The value here is adjusted for midi_ratelimit_per_packet().)
+	 * (The value here is adjusted for amdtp_midi_ratelimit_per_packet().)
 	 */
 	s->midi_fifo_limit = rate - MIDI_BYTES_PER_SECOND * s->syt_interval + 1;
 }
@@ -490,7 +495,7 @@ static void amdtp_fill_pcm_silence(struct amdtp_stream *s,
  * fractional values, the values in midi_fifo_used[] are measured in bytes
  * multiplied by the sample rate.
  */
-static bool midi_ratelimit_per_packet(struct amdtp_stream *s, unsigned int port)
+bool amdtp_midi_ratelimit_per_packet(struct amdtp_stream *s, unsigned int port)
 {
 	int used;
 
@@ -504,11 +509,13 @@ static bool midi_ratelimit_per_packet(struct amdtp_stream *s, unsigned int port)
 
 	return used < s->midi_fifo_limit;
 }
+EXPORT_SYMBOL(amdtp_midi_ratelimit_per_packet);
 
-static void midi_rate_use_one_byte(struct amdtp_stream *s, unsigned int port)
+void amdtp_midi_rate_use_one_byte(struct amdtp_stream *s, unsigned int port)
 {
 	s->midi_fifo_used[port] += amdtp_rate_table[s->sfc];
 }
+EXPORT_SYMBOL(amdtp_midi_rate_use_one_byte);
 
 static void amdtp_fill_midi(struct amdtp_stream *s,
 			    __be32 *buffer, unsigned int frames)
@@ -521,10 +528,10 @@ static void amdtp_fill_midi(struct amdtp_stream *s,
 
 		port = (s->data_block_counter + f) % 8;
 		if (f < MAX_MIDI_RX_BLOCKS &&
-		    midi_ratelimit_per_packet(s, port) &&
+		    amdtp_midi_ratelimit_per_packet(s, port) &&
 		    s->midi[port] != NULL &&
 		    snd_rawmidi_transmit(s->midi[port], &b[1], 1) == 1) {
-			midi_rate_use_one_byte(s, port);
+			amdtp_midi_rate_use_one_byte(s, port);
 			b[0] = 0x81;
 		} else {
 			b[0] = 0x80;
@@ -662,7 +669,7 @@ static void handle_out_packet(struct amdtp_stream *s, unsigned int syt)
 	else
 		amdtp_fill_pcm_silence(s, buffer, data_blocks);
 	if (s->midi_ports)
-		amdtp_fill_midi(s, buffer, data_blocks);
+		s->transfer_midi(s, buffer, data_blocks);
 
 	s->data_block_counter = (s->data_block_counter + data_blocks) & 0xff;
 
@@ -760,7 +767,7 @@ static void handle_in_packet(struct amdtp_stream *s,
 			s->transfer_samples(s, pcm, buffer, data_blocks);
 
 		if (s->midi_ports)
-			amdtp_pull_midi(s, buffer, data_blocks);
+			s->transfer_midi(s, buffer, data_blocks);
 	}
 
 	if (s->flags & CIP_DBC_IS_END_EVENT)
@@ -925,6 +932,15 @@ 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;
 
+	/* Confirm MIDI callback function. */
+	if (s->transfer_midi == NULL) {
+		if (s->direction == AMDTP_OUT_STREAM)
+			s->transfer_midi = amdtp_fill_midi;
+		else
+			s->transfer_midi = amdtp_pull_midi;
+	}
+
+
 	/* initialize packet buffer */
 	if (s->direction == AMDTP_IN_STREAM) {
 		dir = DMA_FROM_DEVICE;
diff --git a/sound/firewire/amdtp.h b/sound/firewire/amdtp.h
index 8a03a91..3999376 100644
--- a/sound/firewire/amdtp.h
+++ b/sound/firewire/amdtp.h
@@ -124,6 +124,8 @@ struct amdtp_stream {
 				 struct snd_pcm_substream *pcm,
 				 __be32 *buffer, unsigned int frames);
 	u8 pcm_positions[AMDTP_MAX_CHANNELS_FOR_PCM];
+	void (*transfer_midi)(struct amdtp_stream *s,
+			      __be32 *buffer, unsigned int frame);
 	u8 midi_position;
 
 	unsigned int syt_interval;
@@ -185,6 +187,9 @@ void amdtp_stream_pcm_abort(struct amdtp_stream *s);
 extern const unsigned int amdtp_syt_intervals[CIP_SFC_COUNT];
 extern const unsigned int amdtp_rate_table[CIP_SFC_COUNT];
 
+bool amdtp_midi_ratelimit_per_packet(struct amdtp_stream *s, unsigned int port);
+void amdtp_midi_rate_use_one_byte(struct amdtp_stream *s, unsigned int port);
+
 /**
  * amdtp_stream_running - check stream is running or not
  * @s: the AMDTP stream
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 10/11] digi00x: improve MIDI capture/playback
  2015-03-15 16:00 [RFC][PATCH 00/11] digi00x: new driver for Digidesign 002/003 family Takashi Sakamoto
                   ` (8 preceding siblings ...)
  2015-03-15 16:01 ` [PATCH 09/11] ALSA: firewire-lib: allows to implement external MIDI callback function Takashi Sakamoto
@ 2015-03-15 16:01 ` Takashi Sakamoto
  2015-03-15 16:01 ` [PATCH 11/11] ALSA: digi00x: apply double-oh-three algorism to multiplex PCM samples Takashi Sakamoto
  10 siblings, 0 replies; 40+ messages in thread
From: Takashi Sakamoto @ 2015-03-15 16:01 UTC (permalink / raw)
  To: clemens, tiwai, robin, damien; +Cc: alsa-devel, ffado-devel

Digi 002/003 family are not conformant to IEC 61883-6:2005 or MMA/AMEI
RP-027. In fact, they uses AM824 format data, but data channel includes
port number in its LSB. The MSB is always 0x80, even if the data channel
includes any MIDI messages. As a result, every MIDI conformant data
channel can transfer maximum 2 bytes.

This commit adds own callback functions to handle this quirk.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/digi00x/digi00x-protocol.c | 48 +++++++++++++++++++++++++++++++
 sound/firewire/digi00x/digi00x-stream.c   |  3 ++
 sound/firewire/digi00x/digi00x.h          |  4 +++
 3 files changed, 55 insertions(+)

diff --git a/sound/firewire/digi00x/digi00x-protocol.c b/sound/firewire/digi00x/digi00x-protocol.c
index 3e5b3bec..4dd373d 100644
--- a/sound/firewire/digi00x/digi00x-protocol.c
+++ b/sound/firewire/digi00x/digi00x-protocol.c
@@ -8,6 +8,54 @@
 
 #include "digi00x.h"
 
+void snd_dg00x_protocol_fill_midi(struct amdtp_stream *s,
+				  __be32 *buffer, unsigned int frames)
+{
+	unsigned int f, port;
+	u8 *b;
+
+	for (f = 0; f < frames; f++) {
+		port = (s->data_block_counter + f) % 4;
+		b = (u8 *)&buffer[s->midi_position];
+
+		/*
+		 * The device allows to transfer MIDI messages by maximum two
+		 * bytes per data channel. But this module transfers one byte
+		 * one time because MIDI data rate is quite lower than IEEE
+		 * 1394 bus data rate.
+		 */
+		if (amdtp_midi_ratelimit_per_packet(s, port) &&
+		    s->midi[port] != NULL &&
+		    snd_rawmidi_transmit(s->midi[port], &b[1], 1) == 1) {
+			amdtp_midi_rate_use_one_byte(s, port);
+			b[3] = 0x01 | (0x10 << port);
+		} else {
+			b[1] = 0;
+			b[3] = 0;
+		}
+		b[0] = 0x80;
+		b[2] = 0;
+
+		buffer += s->data_block_quadlets;
+	}
+}
+
+void snd_dg00x_protocol_pull_midi(struct amdtp_stream *s,
+				  __be32 *buffer, unsigned int frames)
+{
+	unsigned int f;
+	u8 *b;
+
+	for (f = 0; f < frames; f++) {
+		b = (u8 *)&buffer[s->midi_position];
+
+		if (s->midi[0] && (b[3] > 0))
+			snd_rawmidi_receive(s->midi[0], b + 1, b[3]);
+
+		buffer += s->data_block_quadlets;
+	}
+}
+
 struct workqueue_struct *midi_wq;
 
 static void send_midi_control(struct work_struct *work)
diff --git a/sound/firewire/digi00x/digi00x-stream.c b/sound/firewire/digi00x/digi00x-stream.c
index 92eb86d..410b971 100644
--- a/sound/firewire/digi00x/digi00x-stream.c
+++ b/sound/firewire/digi00x/digi00x-stream.c
@@ -204,6 +204,9 @@ static int keep_resources(struct snd_dg00x *dg00x, unsigned int rate)
 	dg00x->rx_stream.midi_position = 0;
 	dg00x->tx_stream.midi_position = 0;
 
+	dg00x->rx_stream.transfer_midi = snd_dg00x_protocol_fill_midi;
+	dg00x->tx_stream.transfer_midi = snd_dg00x_protocol_pull_midi;
+
 	return 0;
 error:
 	release_resources(dg00x);
diff --git a/sound/firewire/digi00x/digi00x.h b/sound/firewire/digi00x/digi00x.h
index 20b178f..a658f44 100644
--- a/sound/firewire/digi00x/digi00x.h
+++ b/sound/firewire/digi00x/digi00x.h
@@ -84,6 +84,10 @@ enum snd_dg00x_optical_mode {
 	SND_DG00X_OPTICAL_MODE_SPDIF,
 };
 
+void snd_dg00x_protocol_fill_midi(struct amdtp_stream *s,
+				  __be32 *buffer, unsigned int frames);
+void snd_dg00x_protocol_pull_midi(struct amdtp_stream *s,
+				  __be32 *buffer, unsigned int frames);
 void snd_dg00x_protocol_queue_midi_message(struct snd_dg00x *dg00x);
 int snd_dg00x_protocol_add_instance(struct snd_dg00x *dg00x);
 void snd_dg00x_protocol_remove_instance(struct snd_dg00x *dg00x);
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* [PATCH 11/11] ALSA: digi00x: apply double-oh-three algorism to multiplex PCM samples
  2015-03-15 16:00 [RFC][PATCH 00/11] digi00x: new driver for Digidesign 002/003 family Takashi Sakamoto
                   ` (9 preceding siblings ...)
  2015-03-15 16:01 ` [PATCH 10/11] digi00x: improve MIDI capture/playback Takashi Sakamoto
@ 2015-03-15 16:01 ` Takashi Sakamoto
  2015-03-16 11:39   ` Damien Zammit
  2015-03-16 14:25   ` Robin Gareus
  10 siblings, 2 replies; 40+ messages in thread
From: Takashi Sakamoto @ 2015-03-15 16:01 UTC (permalink / raw)
  To: clemens, tiwai, robin, damien; +Cc: alsa-devel, ffado-devel

Digi 002/003 family uses own way to multiplex PCM samples into data
blocks in CIP payload for received stream, thus AMDTP-conformant
implementation causes noisy sound.

This commit applies double-oh-three algorism, which discovered by Robin
Gareus and Damien Zammit in 2012.

As long as I tested, this patch still has disorder that:
 * 1st PCM channel still causes noise in 2nd PCM channel.
 * At 88.2/96.0kHz, any PCM channels still causes noise in the other PCM
   channels.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/digi00x/digi00x-protocol.c | 113 ++++++++++++++++++++++++++++++
 sound/firewire/digi00x/digi00x-stream.c   |   2 +
 sound/firewire/digi00x/digi00x.h          |   3 +
 3 files changed, 118 insertions(+)

diff --git a/sound/firewire/digi00x/digi00x-protocol.c b/sound/firewire/digi00x/digi00x-protocol.c
index 4dd373d..ac092cb 100644
--- a/sound/firewire/digi00x/digi00x-protocol.c
+++ b/sound/firewire/digi00x/digi00x-protocol.c
@@ -2,12 +2,125 @@
  * digi00x-protocol.c - a part of driver for Digidesign Digi 002/003 family
  *
  * Copyright (c) 2014-2015 Takashi Sakamoto
+ * Copyright (C) 2012 Robin Gareus <robin@gareus.org>
+ * Copyright (C) 2012 Damien Zammit <damien@zamaudio.com>
  *
  * Licensed under the terms of the GNU General Public License, version 2.
  */
 
 #include "digi00x.h"
 
+/*
+ * The double-oh-three algorism is invented by Robin Gareus and Damien Zammit
+ * in 2012, with reverse-engineering for Digi 003 Rack.
+ */
+
+struct dot_state {
+	__u8 carry;
+	__u8 idx;
+	unsigned int off;
+};
+
+#define BYTE_PER_SAMPLE (4)
+#define MAGIC_DOT_BYTE (2)
+
+#define MAGIC_BYTE_OFF(x) (((x) * BYTE_PER_SAMPLE) + MAGIC_DOT_BYTE)
+
+/*
+ * double-oh-three look up table
+ *
+ * @param idx index byte (audio-sample data) 0x00..0xff
+ * @param off channel offset shift
+ * @return salt to XOR with given data
+ */
+static const __u8 dot_scrt(const __u8 idx, const unsigned int off)
+{
+	/*
+	 * the length of the added pattern only depends on the lower nibble
+	 * of the last non-zero data
+	 */
+	static const __u8 len[16] = {0, 1, 3, 5, 7, 9, 11, 13, 14,
+				     12, 10, 8, 6, 4, 2, 0};
+
+	/*
+	 * the lower nibble of the salt. Interleaved sequence.
+	 * this is walked backwards according to len[]
+	 */
+	static const __u8 nib[15] = {0x8, 0x7, 0x9, 0x6, 0xa, 0x5, 0xb, 0x4,
+				     0xc, 0x3, 0xd, 0x2, 0xe, 0x1, 0xf};
+
+	/* circular list for the salt's hi nibble. */
+	static const __u8 hir[15] = {0x0, 0x6, 0xf, 0x8, 0x7, 0x5, 0x3, 0x4,
+				     0xc, 0xd, 0xe, 0x1, 0x2, 0xb, 0xa};
+
+	/*
+	 * start offset for upper nibble mapping.
+	 * note: 9 is /special/. In the case where the high nibble == 0x9,
+	 * hir[] is not used and - coincidentally - the salt's hi nibble is
+	 * 0x09 regardless of the offset.
+	 */
+	static const __u8 hio[16] = {0, 11, 12, 6, 7, 5, 1, 4,
+				     3, 0x00, 14, 13, 8, 9, 10, 2};
+
+	const __u8 ln = idx & 0xf;
+	const __u8 hn = (idx >> 4) & 0xf;
+	const __u8 hr = (hn == 0x9) ? 0x9 : hir[(hio[hn] + off) % 15];
+
+	if (len[ln] < off)
+		return 0x00;
+
+	return ((nib[14 + off - len[ln]]) | (hr << 4));
+}
+
+static inline void dot_state_reset(struct dot_state *state)
+{
+	state->carry = 0x00;
+	state->idx   = 0x00;
+	state->off   = 0;
+}
+
+static void dot_encode_step(struct dot_state *state, __be32 *const buffer)
+{
+	__u8 * const data = (__u8 *) buffer;
+
+	if (data[MAGIC_DOT_BYTE] != 0x00) {
+		state->off = 0;
+		state->idx = data[MAGIC_DOT_BYTE] ^ state->carry;
+	}
+	data[MAGIC_DOT_BYTE] ^= state->carry;
+	state->carry = dot_scrt(state->idx, ++(state->off));
+}
+
+void snd_dg00x_protocol_write_s32(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;
+	const u32 *src;
+	static struct dot_state state;
+
+	channels = s->pcm_channels;
+	src = (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) {
+		dot_state_reset(&state);
+
+		for (c = 0; c < channels; ++c) {
+			buffer[s->pcm_positions[c]] =
+					cpu_to_be32((*src >> 8) | 0x40000000);
+			dot_encode_step(&state, &buffer[s->pcm_positions[c]]);
+			src++;
+		}
+
+		buffer += s->data_block_quadlets;
+		if (--remaining_frames == 0)
+			src = (void *)runtime->dma_area;
+	}
+}
+
 void snd_dg00x_protocol_fill_midi(struct amdtp_stream *s,
 				  __be32 *buffer, unsigned int frames)
 {
diff --git a/sound/firewire/digi00x/digi00x-stream.c b/sound/firewire/digi00x/digi00x-stream.c
index 410b971..bc4c88c 100644
--- a/sound/firewire/digi00x/digi00x-stream.c
+++ b/sound/firewire/digi00x/digi00x-stream.c
@@ -204,6 +204,8 @@ static int keep_resources(struct snd_dg00x *dg00x, unsigned int rate)
 	dg00x->rx_stream.midi_position = 0;
 	dg00x->tx_stream.midi_position = 0;
 
+	/* Apply doubleOhThree algorism. */
+	dg00x->rx_stream.transfer_samples = snd_dg00x_protocol_write_s32;
 	dg00x->rx_stream.transfer_midi = snd_dg00x_protocol_fill_midi;
 	dg00x->tx_stream.transfer_midi = snd_dg00x_protocol_pull_midi;
 
diff --git a/sound/firewire/digi00x/digi00x.h b/sound/firewire/digi00x/digi00x.h
index a658f44..07e54fc 100644
--- a/sound/firewire/digi00x/digi00x.h
+++ b/sound/firewire/digi00x/digi00x.h
@@ -84,6 +84,9 @@ enum snd_dg00x_optical_mode {
 	SND_DG00X_OPTICAL_MODE_SPDIF,
 };
 
+void snd_dg00x_protocol_write_s32(struct amdtp_stream *s,
+				  struct snd_pcm_substream *pcm,
+				  __be32 *buffer, unsigned int frames);
 void snd_dg00x_protocol_fill_midi(struct amdtp_stream *s,
 				  __be32 *buffer, unsigned int frames);
 void snd_dg00x_protocol_pull_midi(struct amdtp_stream *s,
-- 
2.1.0

^ permalink raw reply related	[flat|nested] 40+ messages in thread

* Re: [PATCH 11/11] ALSA: digi00x: apply double-oh-three algorism to multiplex PCM samples
  2015-03-15 16:01 ` [PATCH 11/11] ALSA: digi00x: apply double-oh-three algorism to multiplex PCM samples Takashi Sakamoto
@ 2015-03-16 11:39   ` Damien Zammit
  2015-03-16 13:24     ` Takashi Sakamoto
  2015-03-16 14:25   ` Robin Gareus
  1 sibling, 1 reply; 40+ messages in thread
From: Damien Zammit @ 2015-03-16 11:39 UTC (permalink / raw)
  To: Takashi Sakamoto, clemens, tiwai, robin; +Cc: alsa-devel, ffado-devel

On 16/03/15 03:01, Takashi Sakamoto wrote:
> Digi 002/003 family uses own way to multiplex PCM samples into data
> blocks in CIP payload for received stream, thus AMDTP-conformant
> implementation causes noisy sound.
>
> This commit applies double-oh-three algorism, which discovered by Robin
> Gareus and Damien Zammit in 2012.
>
> As long as I tested, this patch still has disorder that:
>  * 1st PCM channel still causes noise in 2nd PCM channel.
>  * At 88.2/96.0kHz, any PCM channels still causes noise in the other PCM
>    channels.

Can you please double check, I dont think snd_dg00x_protocol_write_s32()
is even being called because amdtp.c needs a small change to prevent
overriding the transmit_samples function pointer.

Damien

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 11/11] ALSA: digi00x: apply double-oh-three algorism to multiplex PCM samples
  2015-03-16 11:39   ` Damien Zammit
@ 2015-03-16 13:24     ` Takashi Sakamoto
  0 siblings, 0 replies; 40+ messages in thread
From: Takashi Sakamoto @ 2015-03-16 13:24 UTC (permalink / raw)
  To: Damien Zammit, clemens, tiwai, robin; +Cc: alsa-devel, ffado-devel

Hi Damien,

On Mar 16 2015 20:39, Damien Zammit wrote:
> On 16/03/15 03:01, Takashi Sakamoto wrote:
>> Digi 002/003 family uses own way to multiplex PCM samples into data
>> blocks in CIP payload for received stream, thus AMDTP-conformant
>> implementation causes noisy sound.
>>
>> This commit applies double-oh-three algorism, which discovered by Robin
>> Gareus and Damien Zammit in 2012.
>>
>> As long as I tested, this patch still has disorder that:
>>  * 1st PCM channel still causes noise in 2nd PCM channel.
>>  * At 88.2/96.0kHz, any PCM channels still causes noise in the other PCM
>>    channels.
> 
> Can you please double check, I dont think snd_dg00x_protocol_write_s32()
> is even being called because amdtp.c needs a small change to prevent
> overriding the transmit_samples function pointer.

This line overwrites the default callback function with driver-specific
function every time to start streams, thus the driver-specific function
is used for out-stream.

On Mar 16 2015 01:01, Takashi Sakamoto wrote:
> diff --git a/sound/firewire/digi00x/digi00x-stream.c b/sound/firewire/digi00x/digi00x-stream.c
> index 410b971..bc4c88c 100644
> --- a/sound/firewire/digi00x/digi00x-stream.c
> +++ b/sound/firewire/digi00x/digi00x-stream.c
> @@ -204,6 +204,8 @@ static int keep_resources(struct snd_dg00x *dg00x, unsigned int rate)
>  	dg00x->rx_stream.midi_position = 0;
>  	dg00x->tx_stream.midi_position = 0;
>  
> +	/* Apply doubleOhThree algorism. */
> +	dg00x->rx_stream.transfer_samples = snd_dg00x_protocol_write_s32;
>  	dg00x->rx_stream.transfer_midi = snd_dg00x_protocol_fill_midi;
>  	dg00x->tx_stream.transfer_midi = snd_dg00x_protocol_pull_midi;


Regards

Takashi Sakamoto

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 11/11] ALSA: digi00x: apply double-oh-three algorism to multiplex PCM samples
  2015-03-15 16:01 ` [PATCH 11/11] ALSA: digi00x: apply double-oh-three algorism to multiplex PCM samples Takashi Sakamoto
  2015-03-16 11:39   ` Damien Zammit
@ 2015-03-16 14:25   ` Robin Gareus
  2015-03-16 16:25     ` Takashi Sakamoto
  1 sibling, 1 reply; 40+ messages in thread
From: Robin Gareus @ 2015-03-16 14:25 UTC (permalink / raw)
  To: Takashi Sakamoto; +Cc: tiwai, damien, clemens, alsa-devel, ffado-devel

Hi Takashi,

Many thanks for doing this!

On 03/15/2015 05:01 PM, Takashi Sakamoto wrote:

> +
> +void snd_dg00x_protocol_write_s32(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;
> +	const u32 *src;
> +	static struct dot_state state;

There's no need to declare this as static. The state is per frame.

> +	channels = s->pcm_channels;
> +	src = (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) {

..it is zeroed here, anyway:

> +		dot_state_reset(&state);


> +		for (c = 0; c < channels; ++c) {
> +			buffer[s->pcm_positions[c]] =
> +					cpu_to_be32((*src >> 8) | 0x40000000);
> +			dot_encode_step(&state, &buffer[s->pcm_positions[c]]);
> +			src++;
> +		}
> +
> +		buffer += s->data_block_quadlets;
> +		if (--remaining_frames == 0)
> +			src = (void *)runtime->dma_area;
> +	}
> +}


In any case, the algorithm to xor-encode the digidesign data is not yet
100% correct there. One will need to continue iterating after the last
channel (feeding zero) until the state->carry (dot_scrt()) is 0x00.

The current code here only works only correctly for data on the first 4
chanels (18 [total channels] - 14 [max xor-chain length]).

I'll let Damien elaborate. He has access to a Digidesign interface and
did the protocol-dumps. I only did the clean room reverse-engineering
and maths on the other end.

Cheers!
robin

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 11/11] ALSA: digi00x: apply double-oh-three algorism to multiplex PCM samples
  2015-03-16 14:25   ` Robin Gareus
@ 2015-03-16 16:25     ` Takashi Sakamoto
  2015-03-16 17:13       ` Robin Gareus
  0 siblings, 1 reply; 40+ messages in thread
From: Takashi Sakamoto @ 2015-03-16 16:25 UTC (permalink / raw)
  To: Robin Gareus; +Cc: tiwai, damien, clemens, alsa-devel, ffado-devel

Hi Robin,

Thanks for your comment. I have a necessity to understand your algorithm
for better implementation.

On Mar 16 2015 23:25, Robin Gareus wrote:
>> +void snd_dg00x_protocol_write_s32(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;
>> +	const u32 *src;
>> +	static struct dot_state state;
> 
> There's no need to declare this as static. The state is per frame.
> 
>> +	channels = s->pcm_channels;
>> +	src = (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) {
> 
> ..it is zeroed here, anyway:
> 
>> +		dot_state_reset(&state);

Here, I'm consider about the usage of kernel stack. But for this purpose
it may be better to use the stack because it's never nested calls.

>> +		for (c = 0; c < channels; ++c) {
>> +			buffer[s->pcm_positions[c]] =
>> +					cpu_to_be32((*src >> 8) | 0x40000000);
>> +			dot_encode_step(&state, &buffer[s->pcm_positions[c]]);
>> +			src++;
>> +		}
>> +
>> +		buffer += s->data_block_quadlets;
>> +		if (--remaining_frames == 0)
>> +			src = (void *)runtime->dma_area;
>> +	}
>> +}
> 
> 
> In any case, the algorithm to xor-encode the digidesign data is not yet
> 100% correct there. One will need to continue iterating after the last
> channel (feeding zero) until the state->carry (dot_scrt()) is 0x00.
> 
> The current code here only works only correctly for data on the first 4
> chanels (18 [total channels] - 14 [max xor-chain length]).

Hm. Actually, I can hear better sounds in 1/2 ch as long as I aplayback
to the first 4 ch. When 5 or later channels get PCM samples, I can hear
noisy sound in the 1 ch (maybe 2 or more).


This is an sample of CIP packet which Windows driver transmit to Digi
002 Rack, at 96.0 kHz. The first line shows CIP header. One of the other
line shows one data block.

I put 24 bit PCM samples into 7th Multi Bit Linear Audio (MBLA) data
channel (8th data channel). The other channels includes zero samples
except for MIDI conformant data channel (first data channel).

020B0090 9004E400
80000000 40005100 40003F00 40000000 40000000 40000000 40000000 4011C5E4
4000DB00 4000E400 40001C00
80000000 40002300 4000BD00 4000A200 40000E00 40006100 4000FF00 40116AFC
4000F500 40008B00 40007400
80000000 40005C00 40003300 40004D00 4000C200 4000DE00 4000E100 4011DDE1
4000E200 40001E00 40002100
80000000 4000BF00 40000000 40000000 40000000 40000000 40000000 4012DF19
40000000 40000000 40000000
80000000 40000000 40000000 40000000 40000000 40000000 40000000 40146531
4000FB00 40008400 40007C00
80000000 40005300 40003D00 40004200 4000CE00 4000D100 4000EF00 4015B003
40000000 40000000 40000000
80000000 40000000 40000000 40000000 40000000 40000000 40000000 40162CFB
4000B300 4000AD00 40000200
80000000 40006E00 4000F100 40008F00 40000000 40000000 40000000 4015C4F8
4000DC00 4000E300 40001D00
80000000 40002200 4000BE00 4000A100 40000F00 40000000 40000000 4014EC69
40001300 40002D00 4000B200
80000000 4000AE00 40000100 40006F00 40000000 40000000 40000000 40143EE8
40004100 4000CF00 40000000
80000000 40000000 40000000 40000000 40000000 40000000 40000000 401408D0
40006700 4000F900 40008600
80000000 40007A00 40005500 40003B00 40004400 4000CC00 4000D300 4014C0B6
40000000 40000000 40000000
80000000 40000000 40000000 40000000 40000000 40000000 40000000 40146A74
4000F500 40008B00 40007400
80000000 40005C00 40003300 40004D00 4000C200 4000DE00 4000E100 40148837
40007700 40005900 40003600
80000000 40004A00 4000C500 4000DB00 4000E400 40001C00 40002300 401414CA
40002C00 4000B300 4000AD00
80000000 40000200 40006E00 4000F100 40008F00 40000000 40000000 401493B0
40009D00 40009200 40009E00

We can see the data in 7th MBLA data channel influences data in next
data block (data block is represented as 'frame' in driver code). The
pattern is what you discovered. In my understanding, this is the lack of
my implementation.

Do you mean this issue?


Thanks

Takashi Sakamoto

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 11/11] ALSA: digi00x: apply double-oh-three algorism to multiplex PCM samples
  2015-03-16 16:25     ` Takashi Sakamoto
@ 2015-03-16 17:13       ` Robin Gareus
  2015-03-16 22:47         ` Takashi Sakamoto
  0 siblings, 1 reply; 40+ messages in thread
From: Robin Gareus @ 2015-03-16 17:13 UTC (permalink / raw)
  To: Takashi Sakamoto; +Cc: tiwai, damien, clemens, alsa-devel, ffado-devel

On 03/16/2015 05:25 PM, Takashi Sakamoto wrote:
[..]
> 
> We can see the data in 7th MBLA data channel influences data in next
> data block (data block is represented as 'frame' in driver code). The
> pattern is what you discovered. In my understanding, this is the lack of
> my implementation.
> 
> Do you mean this issue?
> 

yes, precisely.

Though from the information Damien sent me it looked like it wraps
around in the current frame, rather then progress to the next..

Anyway, in this case the original code at
https://github.com/x42/003amdtp is also wrong and the driver will have
to allocate space for the state and retain it for subsequent calls.
Using a static on the stack won't work in case someone has multiple of
those devices.

best,
robin

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 11/11] ALSA: digi00x: apply double-oh-three algorism to multiplex PCM samples
  2015-03-16 17:13       ` Robin Gareus
@ 2015-03-16 22:47         ` Takashi Sakamoto
  2015-03-17 13:37           ` Robin Gareus
  0 siblings, 1 reply; 40+ messages in thread
From: Takashi Sakamoto @ 2015-03-16 22:47 UTC (permalink / raw)
  To: Robin Gareus; +Cc: tiwai, damien, clemens, alsa-devel, ffado-devel

Hi Robin,

On May 17 2015 02:13, Robin Gareus wrote:
> On 03/16/2015 05:25 PM, Takashi Sakamoto wrote:
> [..]
>>
>> We can see the data in 7th MBLA data channel influences data in next
>> data block (data block is represented as 'frame' in driver code). The
>> pattern is what you discovered. In my understanding, this is the lack of
>> my implementation.
>>
>> Do you mean this issue?
>>
> 
> yes, precisely.

OK. Thanks for your confirmation.

> Though from the information Damien sent me it looked like it wraps
> around in the current frame, rather then progress to the next..
> 
> Anyway, in this case the original code at
> https://github.com/x42/003amdtp is also wrong and the driver will have
> to allocate space for the state and retain it for subsequent calls.

This idea may solve both issues that 'per-data block' and 'per-packet'.
I describe the detail later.

> Using a static on the stack won't work in case someone has multiple of
> those devices.

Oops, exactly. I forgot this case... Thanks.


This is another sample, with the last four data blocks in a CIP and the
first four data blocks in next CIP. You can see the continuous value in
data-block-counter (dbc) field in each CIP header.

020B00F0 9004D000
...
80000000 4000DA00 4000E500 40001B00 40002400 4000BC00 4000A300 40F04E1C
4000C100 4000DF00 40000000
80000000 40000000 40000000 40000000 40000000 40000000 40000000 40F0530D
40003D00 40004200 4000CE00
80000000 4000D100 4000EF00 40000000 40000000 40000000 40000000 40F0831C
40007D00 40005200 40003E00
80000000 40004100 4000CF00 40000000 40000000 40000000 40000000 40F0E477
40001C00 40002300 4000BD00

020B0000 9004E400
80000000 4000A200 40000E00 40006100 4000FF00 40000000 40000000 40F15FC1
40000000 40000000 40000000
80000000 40000000 40000000 40000000 40000000 40000000 40000000 40F1C387
4000DD00 4000E200 40001E00
80000000 40002100 4000BF00 40000000 40000000 40000000 40000000 40F1DDE5
4000E200 40001E00 40002100
80000000 4000BF00 40000000 40000000 40000000 40000000 40000000 40F19742
40009900 40009600 40009A00
...

We can see the pattern is carried to the data block in next packet. But
current implementation is not good for this case.


Regards

Takashi Sakamoto

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 11/11] ALSA: digi00x: apply double-oh-three algorism to multiplex PCM samples
  2015-03-16 22:47         ` Takashi Sakamoto
@ 2015-03-17 13:37           ` Robin Gareus
  2015-03-17 13:49             ` Damien Zammit
  0 siblings, 1 reply; 40+ messages in thread
From: Robin Gareus @ 2015-03-17 13:37 UTC (permalink / raw)
  To: Takashi Sakamoto; +Cc: tiwai, damien, clemens, alsa-devel, ffado-devel

On 03/16/2015 11:47 PM, Takashi Sakamoto wrote:

Hi Takashi,

> This is another sample, with the last four data blocks in a CIP and the
> first four data blocks in next CIP. You can see the continuous value in
> data-block-counter (dbc) field in each CIP header.
> 

[snip]

> We can see the pattern is carried to the data block in next packet. But
> current implementation is not good for this case.


Indeed, thanks for the heads up.

I've just updated https://github.com/x42/003amdtp/ accordingly.

You'll have to retain the state and reset it on open/re-start but that's
part of driver integration.

Cheers!
robin

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 11/11] ALSA: digi00x: apply double-oh-three algorism to multiplex PCM samples
  2015-03-17 13:37           ` Robin Gareus
@ 2015-03-17 13:49             ` Damien Zammit
  2015-03-18  1:06               ` Takashi Sakamoto
  0 siblings, 1 reply; 40+ messages in thread
From: Damien Zammit @ 2015-03-17 13:49 UTC (permalink / raw)
  To: Robin Gareus, Takashi Sakamoto; +Cc: tiwai, alsa-devel, clemens, ffado-devel

Hi Takashi, others,

Can we include my mixer control into the driver?
I think it is important to include the clock source selector switch as
an alsamixer setting.  Every other serious soundcard allows clock source
selection.

I tested 003R+ with Robin's suggestion of hacking the state to be
statically initialised to zero (which won't work for multi devices),
and the results were great: dead silence on channel 1 when playing into
channel 18.  This small hack means that Robin's update to 003amdtp
should work flawlessly when integrated into the driver properly.

Takashi:  I am also reporting that ADAT sync, SPDIF sync both work using
my clock source selector control.  In fact, without the sync to external
device, there are many dropouts in the 1394 streams.
How can we address this issue?

Regards,
Damien

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 11/11] ALSA: digi00x: apply double-oh-three algorism to multiplex PCM samples
  2015-03-17 13:49             ` Damien Zammit
@ 2015-03-18  1:06               ` Takashi Sakamoto
  2015-03-19  5:18                 ` Damien Zammit
  2015-03-19 13:59                 ` Robin Gareus
  0 siblings, 2 replies; 40+ messages in thread
From: Takashi Sakamoto @ 2015-03-18  1:06 UTC (permalink / raw)
  To: Damien Zammit, Robin Gareus; +Cc: tiwai, alsa-devel, clemens, ffado-devel



On 2015年03月17日 22:49, Damien Zammit wrote:
> Can we include my mixer control into the driver?

Depends on the reason.

> I think it is important to include the clock source selector switch as
> an alsamixer setting.

Please explain the importance, especially the reason to include the 
selector into kernel code instead of userspace application.

> Every other serious soundcard allows clock source selection.

Over generalization.

Some ALSA drivers still expect userspace applications to implement this 
functionality.

> I tested 003R+ with Robin's suggestion of hacking the state to be
> statically initialised to zero (which won't work for multi devices),
> and the results were great: dead silence on channel 1 when playing into
> channel 18.  This small hack means that Robin's update to 003amdtp
> should work flawlessly when integrated into the driver properly.

What we need is to test Robin's new code thoroughly, not include extra 
functionality such as clock source selector unrelated to streaming 
functionality.

> Takashi:  I am also reporting that ADAT sync, SPDIF sync both work using
> my clock source selector control.  In fact, without the sync to external
> device, there are many dropouts in the 1394 streams.
> How can we address this issue?

In this case, the kernel driver should return error code to userspace 
application. If the driver cannot handle any in-packets after starting 
transmitted stream, it detects timeout (500msec) and return -ETIMEDOUT. 
If the in-packets include discontinuity of the value in dbc field of CIP 
header, ALSA AMDTP implementation detects it and stop transmitted 
stream, then the driver also returns -ETIMEDOUT.


Regards

Takashi Sakamoto
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 11/11] ALSA: digi00x: apply double-oh-three algorism to multiplex PCM samples
  2015-03-18  1:06               ` Takashi Sakamoto
@ 2015-03-19  5:18                 ` Damien Zammit
  2015-03-19 13:59                 ` Robin Gareus
  1 sibling, 0 replies; 40+ messages in thread
From: Damien Zammit @ 2015-03-19  5:18 UTC (permalink / raw)
  To: Takashi Sakamoto, Robin Gareus; +Cc: tiwai, alsa-devel, clemens, ffado-devel

On 18/03/15 12:06, Takashi Sakamoto wrote:
> On 2015年03月17日 22:49, Damien Zammit wrote:
>> Can we include my mixer control into the driver?
> 
> Depends on the reason.
The reason for allowing the mixer clock source control in the kernel is
because it is *core* functionality to be able to decide where the sound
card gets its clock from.  It would be cumbersome and awkward if users
had to fire up an external userspace program just to configure the clock
source!  The only reason some sound cards have external userspace
programs to configure these extras is because no one has bothered to
implement them in the kernel yet. (Well that is my opinion).

>> I tested 003R+ with Robin's suggestion of hacking the state to be
>> statically initialised to zero (which won't work for multi devices),
>> and the results were great: dead silence on channel 1 when playing into
>> channel 18.  This small hack means that Robin's update to 003amdtp
>> should work flawlessly when integrated into the driver properly.
> 
> What we need is to test Robin's new code thoroughly, not include extra
> functionality such as clock source selector unrelated to streaming
> functionality.
Takashi, I agree that my clock source mixer control is a separate issue,
but please don't underestimate the work Robin and I have done with
003amdtp:  We are informing you that it now works correctly for all
channels, so long as it is integrated into the driver correctly.  We
know this because we tested it thoroughly with a bus analyser to verify
that the expected patterns were being played into the device and I used
my ears to listen to the pure silence on the other channels when the
magic bit pattern was being played into the device, as well as using a
program to monitor the levels (zero).

>> Takashi:  I am also reporting that ADAT sync, SPDIF sync both work using
>> my clock source selector control.  In fact, without the sync to external
>> device, there are many dropouts in the 1394 streams.
>> How can we address this issue?
> In this case, the kernel driver should return error code to userspace
> application. If the driver cannot handle any in-packets after starting
> transmitted stream, it detects timeout (500msec) and return -ETIMEDOUT.
I would not have been able to notice the difference in dropouts without
my mixer control enabled, so I think from a development point of view it
is useful to include the mixer control in-kernel. (another reason).

> If the in-packets include discontinuity of the value in dbc field of CIP
> header, ALSA AMDTP implementation detects it and stop transmitted
> stream, then the driver also returns -ETIMEDOUT.
Takashi, you have provided here the conditions for which -ETIMEDOUT
occurs.  I am asking how can we prevent the dropouts, not how it occurs.
Perhaps the dbc field is irrelevant for 00x ?  I don't know, please let
me know if there is anything I can dump for you to help find out.

Damien
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 11/11] ALSA: digi00x: apply double-oh-three algorism to multiplex PCM samples
  2015-03-18  1:06               ` Takashi Sakamoto
  2015-03-19  5:18                 ` Damien Zammit
@ 2015-03-19 13:59                 ` Robin Gareus
  2015-03-19 22:46                   ` Takashi Sakamoto
  1 sibling, 1 reply; 40+ messages in thread
From: Robin Gareus @ 2015-03-19 13:59 UTC (permalink / raw)
  To: Takashi Sakamoto; +Cc: tiwai, alsa-devel, clemens, ffado-devel, Damien Zammit

On 03/18/2015 02:06 AM, Takashi Sakamoto wrote:
> 
> 
> On 2015年03月17日 22:49, Damien Zammit wrote:
>> Can we include my mixer control into the driver?
> 
> Depends on the reason.
> 
>> I think it is important to include the clock source selector switch as
>> an alsamixer setting.
> 
> Please explain the importance, especially the reason to include the
> selector into kernel code instead of userspace application.

Can one access the device from userspace while the kernel is using it to
stream audio or midi ("Device or resource busy") ?

This is one of the reasons, why USB soundcards expose all settings
though the alsa mixer interface. It also offers a standardized interface
for userspace apps to build on top and a method to directly link mixer
to soundcard for systems with multiple soundcards.

How do you envisage to handle this?

2c,
robin

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 11/11] ALSA: digi00x: apply double-oh-three algorism to multiplex PCM samples
  2015-03-19 13:59                 ` Robin Gareus
@ 2015-03-19 22:46                   ` Takashi Sakamoto
  2015-03-19 22:51                     ` Takashi Sakamoto
  2015-03-20 12:05                     ` firewire mixer interface -- was " Robin Gareus
  0 siblings, 2 replies; 40+ messages in thread
From: Takashi Sakamoto @ 2015-03-19 22:46 UTC (permalink / raw)
  To: Robin Gareus; +Cc: tiwai, alsa-devel, clemens, ffado-devel, Damien Zammit

Robin and Damien,

On Mar 19 2015 22:59, Robin Gareus wrote:
> On 03/18/2015 02:06 AM, Takashi Sakamoto wrote:
>> Please explain the importance, especially the reason to include the
>> selector into kernel code instead of userspace application.
> 
> Can one access the device from userspace while the kernel is using it to
> stream audio or midi ("Device or resource busy") ?

I can't understand what character devices you mention about. If you
mention about ALSA PCM character devices (usually
/dev/snd/pcmC%uD%d[p|c]), it's yes. In fact, one ALSA application can
use ALSA PCM character device for one direction (playback/capture). So
the access by any other applications causes -EBUSY. (here, I ignore
alsa-lib's PCM direct mixing layer.)

But, when you utilize kernel control functionality, the character device
is not PCM one, it's Control one (/dev/snd/controlC%u).

By the way, we can program any userspace application via FireWire
character device (/dev/fw*). Actually, I utilize firewire-request in
Linux FireWire utilities (former known as jujuutils), and write my own
I/O library, libhinawa.

Linux FireWire utilities
https://github.com/cladisch/linux-firewire-utils

libhinawa
http://mailman.alsa-project.org/pipermail/alsa-devel/2015-January/086969.html

For example, to control the clock selector of Digi 002/003 family, we
just execute this command with firewire-request.

$ ./firewire-request /dev/fw1 write 0xffffe0000118 0x0000000[0|1|2|3]

We can perform to control them from userspace.

> This is one of the reasons, why USB soundcards expose all settings
> though the alsa mixer interface. It also offers a standardized interface
> for userspace apps to build on top and a method to directly link mixer
> to soundcard for systems with multiple soundcards.
> 
> How do you envisage to handle this?

These USB-connected sound devices basically tells their control
capabilities by USB descriptor information. This mechanism is
standardized and included in USB specification. Thus single parser has a
meaning.

On the other hand, IEEE 1394 bus-connected sound devices implements its
own way to tell their control capabilities and model-specific way to
perform control. Thus we should prepare for model-specific parsers. The
idea to include these parsers into kernel-space increases maintaining
efforts.

(Actually, USB-connected sound devices includes vendor-specific
interface. Such interfaces require own codes and snd-usb-audio includes
these code. You can see such codes in sound/usb/mixer_quirks.c and the
other USB Audio device class driver codes.)

In an aspect of user experience, I cannot find any differences between
using alsamixer (or amixer) and using specific-application. ALSA PCM
character devices, ALSA Control character devices and Linux FireWire
character devices are completely different and users don't mind the
differences. What the users' want is to control their physical devices,
to consider about to which character devices they access.


Regards

Takashi Sakamoto

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [PATCH 11/11] ALSA: digi00x: apply double-oh-three algorism to multiplex PCM samples
  2015-03-19 22:46                   ` Takashi Sakamoto
@ 2015-03-19 22:51                     ` Takashi Sakamoto
  2015-03-20 12:05                     ` firewire mixer interface -- was " Robin Gareus
  1 sibling, 0 replies; 40+ messages in thread
From: Takashi Sakamoto @ 2015-03-19 22:51 UTC (permalink / raw)
  To: Robin Gareus; +Cc: tiwai, alsa-devel, clemens, Damien Zammit, ffado-devel

On Mar 20 2015 07:46, Takashi Sakamoto wrote:
> I can't understand what character devices you mention about. If you
> mention about ALSA PCM character devices (usually
> /dev/snd/pcmC%uD%d[p|c]), it's yes. In fact, one ALSA application can
> use ALSA PCM character device for one direction (playback/capture). So
> the access by any other applications causes -EBUSY. (here, I ignore
> alsa-lib's PCM direct mixing layer.)

I'll correct the 'ALSA application' with 'ALSA PCM application'. The
ALSA PCM application and ALSA Control application are different because
they access to different character devices and use different set of
ioctl(2) interfaces or different API set given by alsa-lib.


Regards

Takashi Sakamoto

^ permalink raw reply	[flat|nested] 40+ messages in thread

* firewire mixer interface -- was Re: [PATCH 11/11] ALSA: digi00x: apply double-oh-three algorism to multiplex PCM samples
  2015-03-19 22:46                   ` Takashi Sakamoto
  2015-03-19 22:51                     ` Takashi Sakamoto
@ 2015-03-20 12:05                     ` Robin Gareus
  2015-03-20 13:00                       ` Clemens Ladisch
                                         ` (2 more replies)
  1 sibling, 3 replies; 40+ messages in thread
From: Robin Gareus @ 2015-03-20 12:05 UTC (permalink / raw)
  To: Takashi Sakamoto; +Cc: tiwai, alsa-devel, clemens, ffado-devel, Damien Zammit

On 03/19/2015 11:46 PM, Takashi Sakamoto wrote:

Hi Takashi

Thanks for elaborating.

[..]

> For example, to control the clock selector of Digi 002/003 family, we
> just execute this command with firewire-request.
> 
> $ ./firewire-request /dev/fw1 write 0xffffe0000118 0x0000000[0|1|2|3]

Yes, that's what I was asking about. Can one safely write raw control
messages to /dev/fw* without interfering with ongoing streaming?


Instead interfacing via established protocols /dev/snd/control* or
rather libasound's snd_mixer_t seems like a no-brainer to me.

Are there any control elements on common 1394 devices that cannot be
properly exposed using existing infrastructure?

> On the other hand, IEEE 1394 bus-connected sound devices implements its
> own way to tell their control capabilities and model-specific way to
> perform control. Thus we should prepare for model-specific parsers. The
> idea to include these parsers into kernel-space increases maintaining
> efforts.

Agreed. Most of the heavy lifting should probably be done by libasound.

> In an aspect of user experience, I cannot find any differences between
> using alsamixer (or amixer) and using specific-application.

Uhm. It's a huge difference. There is a whole lot of existing
infrastructure: from Sys-V init.d and/or SystemD save/restore, dbus
hooks, existing mixer GUIs and application integration, not to mention
various language bindings (eg pyalsa).

Linux Audio is already fragmented enough as it stands, adding yet
another interface/toolchain won't help. One might just as well continue
to use ffado. I was under the impression that the whole point of moving
1394 audio into the kernel was to allow seamless integration with the
rest of ALSA.

2c,
robin

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: firewire mixer interface -- was Re: [PATCH 11/11] ALSA: digi00x: apply double-oh-three algorism to multiplex PCM samples
  2015-03-20 12:05                     ` firewire mixer interface -- was " Robin Gareus
@ 2015-03-20 13:00                       ` Clemens Ladisch
  2015-03-20 13:25                       ` Takashi Sakamoto
  2015-03-22  6:11                       ` [FFADO-devel] " Jonathan Woithe
  2 siblings, 0 replies; 40+ messages in thread
From: Clemens Ladisch @ 2015-03-20 13:00 UTC (permalink / raw)
  To: Robin Gareus
  Cc: tiwai, alsa-devel, ffado-devel, Damien Zammit, Takashi Sakamoto

Robin Gareus wrote:
> On 03/19/2015 11:46 PM, Takashi Sakamoto wrote:
>> For example, to control the clock selector of Digi 002/003 family, we
>> just execute this command with firewire-request.
>>
>> $ ./firewire-request /dev/fw1 write 0xffffe0000118 0x0000000[0|1|2|3]
>
> Yes, that's what I was asking about. Can one safely write raw control
> messages to /dev/fw* without interfering with ongoing streaming?

Yes.  If a device did not allow this, or if the mixer accesses would be
part of the audio stream, the driver would have no choice but to
implement this in the kernel.  But this is not the case for most devices.

> Instead interfacing via established protocols /dev/snd/control* or
> rather libasound's snd_mixer_t seems like a no-brainer to me.

It is possible to attach 'virtual' mixer controls to hardware sound
cards.  (This was originally designed for the software volume control.)

The only reason that FFADO did not use this was that there was no
suitable ALSA card instance to attach to.


Regards,
Clemens

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: firewire mixer interface -- was Re: [PATCH 11/11] ALSA: digi00x: apply double-oh-three algorism to multiplex PCM samples
  2015-03-20 12:05                     ` firewire mixer interface -- was " Robin Gareus
  2015-03-20 13:00                       ` Clemens Ladisch
@ 2015-03-20 13:25                       ` Takashi Sakamoto
  2015-03-20 13:35                         ` Takashi Iwai
  2015-03-20 13:55                         ` Damien Zammit
  2015-03-22  6:11                       ` [FFADO-devel] " Jonathan Woithe
  2 siblings, 2 replies; 40+ messages in thread
From: Takashi Sakamoto @ 2015-03-20 13:25 UTC (permalink / raw)
  To: Robin Gareus; +Cc: tiwai, alsa-devel, clemens, Damien Zammit, ffado-devel

Hi Robin,

On Mar 20 2015 21:05, Robin Gareus wrote:
>> $ ./firewire-request /dev/fw1 write 0xffffe0000118 0x0000000[0|1|2|3]
> 
> Yes, that's what I was asking about. Can one safely write raw control
> messages to /dev/fw* without interfering with ongoing streaming?

Any userspace applications can destroy packet streaming which kernel
driver starts, by transaction to streaming-related register.

In current implementation of ALSA firewire stack and Linux FireWire
subsystem, we cannot avoid this.

For example, about Digi 002/003 family, we can destroy kernel streaming
in a way below:
1.write/read PCM samples to ALSA PCM character devices (in most case
done via alsa-lib PCM API)
2.write transaction with 0x00000003 for 0xffffe0000004 to /dev/fw%u.
3.Applications cannot read/write PCM samples again.

In this case, usually, the process receive EIO from ALSA PCM API.

> Instead interfacing via established protocols /dev/snd/control* or
> rather libasound's snd_mixer_t seems like a no-brainer to me.

As long as being able to send transactions via FireWire character
devices, the headache remains, regardless of the place to implement such
control functionality.

> Are there any control elements on common 1394 devices that cannot be
> properly exposed using existing infrastructure?

More explaination, please.

>> On the other hand, IEEE 1394 bus-connected sound devices implements its
>> own way to tell their control capabilities and model-specific way to
>> perform control. Thus we should prepare for model-specific parsers. The
>> idea to include these parsers into kernel-space increases maintaining
>> efforts.
> 
> Agreed. Most of the heavy lifting should probably be done by libasound.

I don't think it possible to argue the other ALSA developers for going
to include such vendor-specific or model-specific huge codes to
alsa-lib... (Except for intel HDA)

>> In an aspect of user experience, I cannot find any differences between
>> using alsamixer (or amixer) and using specific-application.
> 
> Uhm. It's a huge difference. There is a whole lot of existing
> infrastructure: from Sys-V init.d and/or SystemD save/restore, dbus
> hooks, existing mixer GUIs and application integration, not to mention
> various language bindings (eg pyalsa).

Here, I mention about alsa-lib control API to add control elements from
userspace and it's eventing mechanism.
http://www.alsa-project.org/alsa-doc/alsa-lib/group___control.html#gad5f640f1d836b532b1c18d7604a90bad

Regards


Takashi Sakamoto

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: firewire mixer interface -- was Re: [PATCH 11/11] ALSA: digi00x: apply double-oh-three algorism to multiplex PCM samples
  2015-03-20 13:25                       ` Takashi Sakamoto
@ 2015-03-20 13:35                         ` Takashi Iwai
  2015-03-20 13:51                           ` Takashi Sakamoto
  2015-03-20 13:55                         ` Damien Zammit
  1 sibling, 1 reply; 40+ messages in thread
From: Takashi Iwai @ 2015-03-20 13:35 UTC (permalink / raw)
  To: Takashi Sakamoto
  Cc: alsa-devel, Robin Gareus, clemens, Damien Zammit, ffado-devel

At Fri, 20 Mar 2015 22:25:26 +0900,
Takashi Sakamoto wrote:
> 
> >> On the other hand, IEEE 1394 bus-connected sound devices implements its
> >> own way to tell their control capabilities and model-specific way to
> >> perform control. Thus we should prepare for model-specific parsers. The
> >> idea to include these parsers into kernel-space increases maintaining
> >> efforts.
> > 
> > Agreed. Most of the heavy lifting should probably be done by libasound.
> 
> I don't think it possible to argue the other ALSA developers for going
> to include such vendor-specific or model-specific huge codes to
> alsa-lib... (Except for intel HDA)

Why not implementing as a plugin?


Takashi

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: firewire mixer interface -- was Re: [PATCH 11/11] ALSA: digi00x: apply double-oh-three algorism to multiplex PCM samples
  2015-03-20 13:35                         ` Takashi Iwai
@ 2015-03-20 13:51                           ` Takashi Sakamoto
  2015-03-20 14:13                             ` Takashi Iwai
  0 siblings, 1 reply; 40+ messages in thread
From: Takashi Sakamoto @ 2015-03-20 13:51 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: alsa-devel, Robin Gareus, clemens, Damien Zammit, ffado-devel

On Mar 20 2015 22:35, Takashi Iwai wrote:
>> I don't think it possible to argue the other ALSA developers for going
>> to include such vendor-specific or model-specific huge codes to
>> alsa-lib... (Except for intel HDA)
> 
> Why not implementing as a plugin?

As long as I know, we cannot write any configuration to load it for 'hw'
node. On the other hand, when adding any nodes like 'bebob' or 'dice',
they always stay in alsa-lib configuration space even if there're no
actual devices connected.

If my understanding is wrong, please inform it to me.


Regards

Takashi Sakamoto

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: firewire mixer interface -- was Re: [PATCH 11/11] ALSA: digi00x: apply double-oh-three algorism to multiplex PCM samples
  2015-03-20 13:25                       ` Takashi Sakamoto
  2015-03-20 13:35                         ` Takashi Iwai
@ 2015-03-20 13:55                         ` Damien Zammit
  2015-03-20 14:07                           ` Clemens Ladisch
  1 sibling, 1 reply; 40+ messages in thread
From: Damien Zammit @ 2015-03-20 13:55 UTC (permalink / raw)
  To: Takashi Sakamoto, Robin Gareus; +Cc: tiwai, alsa-devel, clemens, ffado-devel

On 21/03/15 00:25, Takashi Sakamoto wrote:
> Any userspace applications can destroy packet streaming which kernel
> driver starts, by transaction to streaming-related register.
> 
> In current implementation of ALSA firewire stack and Linux FireWire
> subsystem, we cannot avoid this.

Does that mean the mixer control will need root permissions to execute
in userspace because the /dev/fwX node is usually owned by root?

Damien

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: firewire mixer interface -- was Re: [PATCH 11/11] ALSA: digi00x: apply double-oh-three algorism to multiplex PCM samples
  2015-03-20 13:55                         ` Damien Zammit
@ 2015-03-20 14:07                           ` Clemens Ladisch
  0 siblings, 0 replies; 40+ messages in thread
From: Clemens Ladisch @ 2015-03-20 14:07 UTC (permalink / raw)
  To: Damien Zammit, Takashi Sakamoto, Robin Gareus
  Cc: tiwai, alsa-devel, ffado-devel

Damien Zammit wrote:
> On 21/03/15 00:25, Takashi Sakamoto wrote:
>> Any userspace applications can destroy packet streaming which kernel
>> driver starts, by transaction to streaming-related register.
>>
>> In current implementation of ALSA firewire stack and Linux FireWire
>> subsystem, we cannot avoid this.
>
> Does that mean the mixer control will need root permissions to execute
> in userspace because the /dev/fwX node is usually owned by root?

/dev/fw* of audio devices typically get a different group to allow FFADO
to run.  Whatever software implements these mixer controls (whether it
ends up called FFADO or not) would run with the same permissions.


Regards,
Clemens

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: firewire mixer interface -- was Re: [PATCH 11/11] ALSA: digi00x: apply double-oh-three algorism to multiplex PCM samples
  2015-03-20 13:51                           ` Takashi Sakamoto
@ 2015-03-20 14:13                             ` Takashi Iwai
  2015-03-20 14:45                               ` Takashi Sakamoto
  0 siblings, 1 reply; 40+ messages in thread
From: Takashi Iwai @ 2015-03-20 14:13 UTC (permalink / raw)
  To: Takashi Sakamoto
  Cc: alsa-devel, Robin Gareus, clemens, Damien Zammit, ffado-devel

At Fri, 20 Mar 2015 22:51:25 +0900,
Takashi Sakamoto wrote:
> 
> On Mar 20 2015 22:35, Takashi Iwai wrote:
> >> I don't think it possible to argue the other ALSA developers for going
> >> to include such vendor-specific or model-specific huge codes to
> >> alsa-lib... (Except for intel HDA)
> > 
> > Why not implementing as a plugin?
> 
> As long as I know, we cannot write any configuration to load it for 'hw'
> node. On the other hand, when adding any nodes like 'bebob' or 'dice',
> they always stay in alsa-lib configuration space even if there're no
> actual devices connected.
> 
> If my understanding is wrong, please inform it to me.

You seem mixing up how to use the plugin setup and how to write the
plugin...  The usage with a plugin might be more complex indeed, but
it's more or less same no matter whether you implement in alsa-lib
itself or implement as an external plugin.


Takashi

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: firewire mixer interface -- was Re: [PATCH 11/11] ALSA: digi00x: apply double-oh-three algorism to multiplex PCM samples
  2015-03-20 14:13                             ` Takashi Iwai
@ 2015-03-20 14:45                               ` Takashi Sakamoto
  2015-03-20 15:01                                 ` Takashi Iwai
  0 siblings, 1 reply; 40+ messages in thread
From: Takashi Sakamoto @ 2015-03-20 14:45 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: alsa-devel, Robin Gareus, clemens, ffado-devel, Damien Zammit

On Mar 20 2015 23:13, Takashi Iwai wrote:
> At Fri, 20 Mar 2015 22:51:25 +0900,
> Takashi Sakamoto wrote:
>>
>> On Mar 20 2015 22:35, Takashi Iwai wrote:
>>>> I don't think it possible to argue the other ALSA developers for going
>>>> to include such vendor-specific or model-specific huge codes to
>>>> alsa-lib... (Except for intel HDA)
>>>
>>> Why not implementing as a plugin?
>>
>> As long as I know, we cannot write any configuration to load it for 'hw'
>> node. On the other hand, when adding any nodes like 'bebob' or 'dice',
>> they always stay in alsa-lib configuration space even if there're no
>> actual devices connected.
>>
>> If my understanding is wrong, please inform it to me.
> 
> You seem mixing up how to use the plugin setup and how to write the
> plugin...  The usage with a plugin might be more complex indeed, but
> it's more or less same no matter whether you implement in alsa-lib
> itself or implement as an external plugin.

Sorry, but I consider about one-step future.

I think it possible to discuss constructively about such plugins for
alsa-plugins, while its usage is not so easy for usual users of FireWire
audio devices. I can imagine to receive much requests about
improvements, then consider about including it to alsa-lib itself. But
this idea may be hard to achieve because of the reasons I describe.

I felt a bit unhappiness about your question and had a logic jump,
sorry. I'm not so tough developer...


Regards

Takashi Sakamoto

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: firewire mixer interface -- was Re: [PATCH 11/11] ALSA: digi00x: apply double-oh-three algorism to multiplex PCM samples
  2015-03-20 14:45                               ` Takashi Sakamoto
@ 2015-03-20 15:01                                 ` Takashi Iwai
  2015-03-21  5:59                                   ` Damien Zammit
  0 siblings, 1 reply; 40+ messages in thread
From: Takashi Iwai @ 2015-03-20 15:01 UTC (permalink / raw)
  To: Takashi Sakamoto
  Cc: alsa-devel, Robin Gareus, clemens, ffado-devel, Damien Zammit

At Fri, 20 Mar 2015 23:45:40 +0900,
Takashi Sakamoto wrote:
> 
> On Mar 20 2015 23:13, Takashi Iwai wrote:
> > At Fri, 20 Mar 2015 22:51:25 +0900,
> > Takashi Sakamoto wrote:
> >>
> >> On Mar 20 2015 22:35, Takashi Iwai wrote:
> >>>> I don't think it possible to argue the other ALSA developers for going
> >>>> to include such vendor-specific or model-specific huge codes to
> >>>> alsa-lib... (Except for intel HDA)
> >>>
> >>> Why not implementing as a plugin?
> >>
> >> As long as I know, we cannot write any configuration to load it for 'hw'
> >> node. On the other hand, when adding any nodes like 'bebob' or 'dice',
> >> they always stay in alsa-lib configuration space even if there're no
> >> actual devices connected.
> >>
> >> If my understanding is wrong, please inform it to me.
> > 
> > You seem mixing up how to use the plugin setup and how to write the
> > plugin...  The usage with a plugin might be more complex indeed, but
> > it's more or less same no matter whether you implement in alsa-lib
> > itself or implement as an external plugin.
> 
> Sorry, but I consider about one-step future.
> 
> I think it possible to discuss constructively about such plugins for
> alsa-plugins, while its usage is not so easy for usual users of FireWire
> audio devices. I can imagine to receive much requests about
> improvements, then consider about including it to alsa-lib itself. But
> this idea may be hard to achieve because of the reasons I describe.

OK, point taken.  Then it's no matter whether plugin or not.  Rather a
question is whether it works out-of-the-box without any extra
configuration, right?

It'd be possible to achieve this in pure alsa-lib config, but it can
be complicated.  The alsa-lib config may choose different sub-config
depending on the hardware component.  The current per-card config
setup is already so, and USB-audio.conf contains yet more per-driver
extra config.

But I don't also push this as the best solution, either.  My comment
was only about the implementation detail *if* we want to implement as
an alsa-lib functionality.  Any better solution is more welcome.

However, what would be the better alternative for the mixer setup
including the clock source selection?  Invoking another mixer program
may annoy people (as least already two people complained :)


Takashi

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: firewire mixer interface -- was Re: [PATCH 11/11] ALSA: digi00x: apply double-oh-three algorism to multiplex PCM samples
  2015-03-20 15:01                                 ` Takashi Iwai
@ 2015-03-21  5:59                                   ` Damien Zammit
  2015-03-22  2:55                                     ` Takashi Sakamoto
  0 siblings, 1 reply; 40+ messages in thread
From: Damien Zammit @ 2015-03-21  5:59 UTC (permalink / raw)
  To: Takashi Iwai, Takashi Sakamoto
  Cc: alsa-devel, Robin Gareus, clemens, ffado-devel



On 21/03/15 02:01, Takashi Iwai wrote:
> OK, point taken.  Then it's no matter whether plugin or not.  Rather a
> question is whether it works out-of-the-box without any extra
> configuration, right?
I think it would be a nice feature if it just worked out of the box like
any other alsa mixer control, and I'm sure many other people who are
currently using FFADO would agree that having everything to control the
sound card in ALSA would be more convenient than what they have now.

Takashi S: can we perhaps focus on getting the streaming working better
with 003amdtp for digi00x and revisit the issue of mixer controls later?
Have you looked at the updated 003amdtp documentation?  I believe Robin
has made suggestions on how to integrate the latest code into the driver.

<snip>

#ifdef EXAMPLE
  // NB. this should not be static (multiple devices)
  // and also re-initialzed on open/close or stream-restart.
  // use digi_state_reset(&state);
  static DigiMagic digistate = {0x00, 0x00, 0};
#endif

</snip>

Damien

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: firewire mixer interface -- was Re: [PATCH 11/11] ALSA: digi00x: apply double-oh-three algorism to multiplex PCM samples
  2015-03-21  5:59                                   ` Damien Zammit
@ 2015-03-22  2:55                                     ` Takashi Sakamoto
  2015-03-22  5:56                                       ` [FFADO-devel] " Jonathan Woithe
  2015-03-24  3:15                                       ` Robin Gareus
  0 siblings, 2 replies; 40+ messages in thread
From: Takashi Sakamoto @ 2015-03-22  2:55 UTC (permalink / raw)
  To: Damien Zammit, Takashi Iwai
  Cc: alsa-devel, Robin Gareus, clemens, ffado-devel

On Mar 21 2015 14:59, Damien Zammit wrote:
> I think it would be a nice feature if it just worked out of the box like
> any other alsa mixer control, and I'm sure many other people who are
> currently using FFADO would agree that having everything to control the
> sound card in ALSA would be more convenient than what they have now.

Over generalization.

Actually ALSA middleware cannot represent control interfaces with the
same level as FFADO achieves, thus your idea can bring no advantages to
FFADO users.

> Takashi S: can we perhaps focus on getting the streaming working better
> with 003amdtp for digi00x and revisit the issue of mixer controls later?

It's what I've suggested.

> Have you looked at the updated 003amdtp documentation?  I believe Robin
> has made suggestions on how to integrate the latest code into the driver.

It's due to what I've reported to Robin. According to his response, I've
judge his codes have never passed any appropriate tests with actual devices.


Regards

Takashi Sakamoto

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [FFADO-devel] firewire mixer interface -- was Re: [PATCH 11/11] ALSA: digi00x: apply double-oh-three algorism to multiplex PCM samples
  2015-03-22  2:55                                     ` Takashi Sakamoto
@ 2015-03-22  5:56                                       ` Jonathan Woithe
  2015-03-24  3:15                                       ` Robin Gareus
  1 sibling, 0 replies; 40+ messages in thread
From: Jonathan Woithe @ 2015-03-22  5:56 UTC (permalink / raw)
  To: Takashi Sakamoto
  Cc: Takashi Iwai, alsa-devel, Robin Gareus, ffado-devel, Damien Zammit

On Sun, Mar 22, 2015 at 11:55:24AM +0900, Takashi Sakamoto wrote:
> On Mar 21 2015 14:59, Damien Zammit wrote:
> > I think it would be a nice feature if it just worked out of the box like
> > any other alsa mixer control, and I'm sure many other people who are
> > currently using FFADO would agree that having everything to control the
> > sound card in ALSA would be more convenient than what they have now.
> 
> Over generalization.
> 
> Actually ALSA middleware cannot represent control interfaces with the
> same level as FFADO achieves, thus your idea can bring no advantages to
> FFADO users.

I've been following this thread but haven't had a chance to respond until
now.  Speaking generally I think Takashi S's comment is a valid
consideration.  While there are some controls on some Firewire audio devices
which could make use of the alsamixer interface, there are other situations
where the alsamixer program and/or API either could not cope or would be
very cumbersome to use.  For these devices having a dedicated mixer-type
application which communicated directly with the audio interface would make
sense.  Mixer controls function independently of the streaming engine in
almost every case, so there is no technical reason why the kernel would need
to be burdened which what could end up being a very large block of code.

I don't see this as a bad thing though.  Complex audio interfaces already
have dedicated mixer applications in alsa-tools.  I don't think the lack of
certain controls via the generic alsamixer hinders the use of these devices.

Regarding device control, there will always be a need to draw a line in the
sand somewhere.  At some point a certain device control becomes secondary to
audio streaming and more to do with signal routing and conditioning in the
device.  The clock source control which Damien mentioned is a case in point. 
Assuming the presence of a clock signal on the selected clock source,
streaming startup doesn't really need to be concerned with the selection of
the clock source.  However, it is clearly related to streaming so it could
be argued that something like this does deserve to be provided with an alsa
API interface of some description - even if it is only for the convenience
of users.  Contrast this to phantom power switches, which have nothing to do
with audio streaming at all.

> It does mean that alsa clients (such as jackd) don't currently provide a
> way to switch clock sources on startup.

Most sound cards don't have multiple clock sources, which is probably the
reason why there is not a standard way to represent such a control via the
alsa API.  That doesn't mean that one couldn't be defined of course, and
doing so would allow clients like jackd to switch clock sources on startup
(something which would be useful for a number of use cases).  The
complicating factor in this is that while many interfaces could represent
the clock source selection as a simple one-of-many selector, some devices
are in reality more complicated than this.  As a result, any generic clock
source selector - if adopted - would have to allow for this to be done in
other ways if needed.

What all this comes down to is that I don't see a problem with the use of
userspace model-specific mixer applications for interfaces which genuinely
need it.  It seems to have worked out fine for envy24control and hdspmixer
for example.  Whether these talk direct to the device or via some alsa API
is probably a device-by-device decision.

If a control/mixer program was required for a given interface I would expect
it to be ultimately included in alsa-tools.  Having the required tools
within the alsa packages would get as close to the "works out the box"
situation as possible.

Regards
  jonathan

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: [FFADO-devel] firewire mixer interface -- was Re: [PATCH 11/11] ALSA: digi00x: apply double-oh-three algorism to multiplex PCM samples
  2015-03-20 12:05                     ` firewire mixer interface -- was " Robin Gareus
  2015-03-20 13:00                       ` Clemens Ladisch
  2015-03-20 13:25                       ` Takashi Sakamoto
@ 2015-03-22  6:11                       ` Jonathan Woithe
  2 siblings, 0 replies; 40+ messages in thread
From: Jonathan Woithe @ 2015-03-22  6:11 UTC (permalink / raw)
  To: Robin Gareus; +Cc: tiwai, alsa-devel, ffado-devel, Takashi Sakamoto

On Fri, Mar 20, 2015 at 01:05:16PM +0100, Robin Gareus wrote:
> I was under the impression that the whole point of moving 1394 audio into
> the kernel was to allow seamless integration with the rest of ALSA.

Speaking as a FFADO developer, the primary reason for pushing FFADO's audio
streaming code into the kernel is for performance and reliability reasons:
it is difficult to maintain the necessary timing when running as a userspace
client of the juju ABI.  Being in kernel brings advantages in this respect. 
The mixer side of things is competely orthogonal to the audio streaming
system in FFADO and was therefore peripheral to this.  As a result the
movement of device control into ALSA wasn't really discussed in great
detail: regardless of whether a kernel or user-space streaming engine is
used, the device control component of FFADO (ffado-mixer) will continue to
be functional.

At least in the first instance the thought was to concentrate on getting the
streaming code into the kernel since that would resolve a vast majority of
performance issues that FFADO faces.  Furthermore, kernel streaming is the
only manditory component needed to allow firewire interfaces to be usable
from "ALSA" applications (presently a program must support JACK to make use
of devices supported by FFADO).  For many users of these interfaces, the
fact that they might have to use something other than alsamixer to control
the interface is completely irrelevant.

The idea was that once in-kernel streaming was operational the longer term
mixer situation could then be looked at if it made sense to change it. 
Whether things work out like this remains to be seen.

Regards
  jonathan

^ permalink raw reply	[flat|nested] 40+ messages in thread

* Re: firewire mixer interface -- was Re: [PATCH 11/11] ALSA: digi00x: apply double-oh-three algorism to multiplex PCM samples
  2015-03-22  2:55                                     ` Takashi Sakamoto
  2015-03-22  5:56                                       ` [FFADO-devel] " Jonathan Woithe
@ 2015-03-24  3:15                                       ` Robin Gareus
  1 sibling, 0 replies; 40+ messages in thread
From: Robin Gareus @ 2015-03-24  3:15 UTC (permalink / raw)
  To: Takashi Sakamoto
  Cc: Takashi Iwai, alsa-devel, clemens, ffado-devel, Damien Zammit

On 03/22/2015 03:55 AM, Takashi Sakamoto wrote:
[..]
> It's due to what I've reported to Robin. According to his response, I've
> judge his codes have never passed any appropriate tests with actual devices.

We did clean-room reverse engineer the protocol and key. I do not own
any digidesign devices (nor any other firewire devices for that matter)
and have never tested it myself on real hardware.

I can only state that the current code works correctly on all the raw
bus data dumps that I have seen so far.

Damien verified the code to work on real hardware and mentioned that in
a prior email and also on
https://github.com/takaswie/snd-firewire-improve/issues/16

Cheers!
robin

^ permalink raw reply	[flat|nested] 40+ messages in thread

end of thread, other threads:[~2015-03-24  3:15 UTC | newest]

Thread overview: 40+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-15 16:00 [RFC][PATCH 00/11] digi00x: new driver for Digidesign 002/003 family Takashi Sakamoto
2015-03-15 16:00 ` [PATCH 01/11] ALSA: digi00x: add skelton for Digi 002/003 device driver Takashi Sakamoto
2015-03-15 16:01 ` [PATCH 02/11] ALSA: digi00x: add streaming functionality Takashi Sakamoto
2015-03-15 16:01 ` [PATCH 03/11] ALSA: digi00x: add proc node for clock status Takashi Sakamoto
2015-03-15 16:01 ` [PATCH 04/11] ALSA: digi00x: add PCM functionality Takashi Sakamoto
2015-03-15 16:01 ` [PATCH 05/11] ALSA: digi00x: add MIDI functionality Takashi Sakamoto
2015-03-15 16:01 ` [PATCH 06/11] ALSA: digi00x: add hwdep interface Takashi Sakamoto
2015-03-15 16:01 ` [PATCH 07/11] ALSA: digi00x: support unknown asynchronous message Takashi Sakamoto
2015-03-15 16:01 ` [PATCH 08/11] ALSA: digi00x: support MIDI ports for device control Takashi Sakamoto
2015-03-15 16:01 ` [PATCH 09/11] ALSA: firewire-lib: allows to implement external MIDI callback function Takashi Sakamoto
2015-03-15 16:01 ` [PATCH 10/11] digi00x: improve MIDI capture/playback Takashi Sakamoto
2015-03-15 16:01 ` [PATCH 11/11] ALSA: digi00x: apply double-oh-three algorism to multiplex PCM samples Takashi Sakamoto
2015-03-16 11:39   ` Damien Zammit
2015-03-16 13:24     ` Takashi Sakamoto
2015-03-16 14:25   ` Robin Gareus
2015-03-16 16:25     ` Takashi Sakamoto
2015-03-16 17:13       ` Robin Gareus
2015-03-16 22:47         ` Takashi Sakamoto
2015-03-17 13:37           ` Robin Gareus
2015-03-17 13:49             ` Damien Zammit
2015-03-18  1:06               ` Takashi Sakamoto
2015-03-19  5:18                 ` Damien Zammit
2015-03-19 13:59                 ` Robin Gareus
2015-03-19 22:46                   ` Takashi Sakamoto
2015-03-19 22:51                     ` Takashi Sakamoto
2015-03-20 12:05                     ` firewire mixer interface -- was " Robin Gareus
2015-03-20 13:00                       ` Clemens Ladisch
2015-03-20 13:25                       ` Takashi Sakamoto
2015-03-20 13:35                         ` Takashi Iwai
2015-03-20 13:51                           ` Takashi Sakamoto
2015-03-20 14:13                             ` Takashi Iwai
2015-03-20 14:45                               ` Takashi Sakamoto
2015-03-20 15:01                                 ` Takashi Iwai
2015-03-21  5:59                                   ` Damien Zammit
2015-03-22  2:55                                     ` Takashi Sakamoto
2015-03-22  5:56                                       ` [FFADO-devel] " Jonathan Woithe
2015-03-24  3:15                                       ` Robin Gareus
2015-03-20 13:55                         ` Damien Zammit
2015-03-20 14:07                           ` Clemens Ladisch
2015-03-22  6:11                       ` [FFADO-devel] " Jonathan Woithe

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.