All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 00/13] ALSA: dice: use cache of stream formats to generate PCM rules/constraints
@ 2018-04-29  6:50 Takashi Sakamoto
  2018-04-29  6:50 ` [PATCH v2 01/13] ALSA: dice: add cache of stream formats Takashi Sakamoto
                   ` (12 more replies)
  0 siblings, 13 replies; 21+ messages in thread
From: Takashi Sakamoto @ 2018-04-29  6:50 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel, ffado-devel

Hi,

This patchset updates my previous one:

[alsa-devel] [PATCH 00/11] ALSA: dice: use cache of stream formats to generate PCM rules/constraints
http://mailman.alsa-project.org/pipermail/alsa-devel/2018-April/134983.html

Once, ALSA dice driver cached stream formats to generate PCM
constraints/rules. The cache is created by changing current
sampling transmission frequency step by step directly at unit
probing process. This feature was dropped because it changes
state of clock on target device thus it's against users'
expectation[1].

However this is not necessarily convenient because reconfiguration
of sampling rate is not available.

TC Applied Technologies (TCAT) has extended protocol for DICE
interface, a.k.a EAP (Extended Application Protocol). In this
extension, drivers can retrieve information of stream formats
without changing state of target unit. Fortunately, this protocol
is supported widely by many models.

This patchset regain the cache to generate PCM constraints/rules
by using this protocol. Userspace applications can reconfigure
sampling rate via ALSA PCM interface.

For units which doesn't support the extension, available set of
sampling rate is still restricted at current sampling transmission
frequency. This is also inconvenient and this patchset introduces a
solution.

Structure for unit entry (struct ieee1394_device_id) has a member
for unit-specific data. This patchset uses it for callback function
to handle hard-coded stream format. In this time, I adopt this way
for some known models produced by TC Electronic and Alesis. If users
find their models still have restrictions in a point of available
sampling transmission frequency, it's preferable to work for this
way.

Changes from v1:
 - rename members for the channel cache.
 - add minor refactoring.
 - fix a bug that sampling transmission frequency is not changed via
   ALSA PCM interface.
 - fix a bug that current stream formats are reused after changing
   sampling transmission frequency.

[1] [alsa-devel] [PATCH 0/6 v2] ALSA: dice: constrain PCM substreams to current sampling transfer frequency
http://mailman.alsa-project.org/pipermail/alsa-devel/2016-February/104065.html
[2] [alsa-devel] [PATCH] ALSA: dice: fix OUI for TC group
http://mailman.alsa-project.org/pipermail/alsa-devel/2018-April/134804.html

Takashi Sakamoto (13):
  ALSA: dice: add cache of stream formats
  ALSA: dice: add 'firewire' directory for proc nodes
  ALSA: dice: add proc node for stream formation
  ALSA: dice: cache stream formats at current mode of sampling
    transmission frequency
  ALSA: dice: add parameters of stream formats for models produced by TC
    Electronic
  ALSA: dice: add parameters of stream formats for models produced by
    Alesis
  ALSA: dice: use extended protocol to detect available stream formats
  ALSA: dice: use cache of stream format to check running stream
  ALSA: dice: add a helper function to restart all of available streams
  ALSA: dice: enable to change current sampling transmission frequency
  ALSA: dice: use stream formats to add MIDI substreams
  ALSA: dice: use cache for PCM constraints and rules
  ALSA: dice: remove local frag of force_two_pcms

 sound/firewire/dice/Makefile            |   3 +-
 sound/firewire/dice/dice-alesis.c       |  48 ++++++
 sound/firewire/dice/dice-extension.c    | 172 ++++++++++++++++++++
 sound/firewire/dice/dice-midi.c         |  23 +--
 sound/firewire/dice/dice-pcm.c          | 230 ++++++++++++++++++---------
 sound/firewire/dice/dice-proc.c         |  70 +++++++-
 sound/firewire/dice/dice-stream.c       | 273 ++++++++++++++++++++++++--------
 sound/firewire/dice/dice-tcelectronic.c | 100 ++++++++++++
 sound/firewire/dice/dice.c              | 139 ++++++++++------
 sound/firewire/dice/dice.h              |  17 +-
 10 files changed, 867 insertions(+), 208 deletions(-)
 create mode 100644 sound/firewire/dice/dice-alesis.c
 create mode 100644 sound/firewire/dice/dice-extension.c
 create mode 100644 sound/firewire/dice/dice-tcelectronic.c

-- 
2.14.1

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

* [PATCH v2 01/13] ALSA: dice: add cache of stream formats
  2018-04-29  6:50 [PATCH v2 00/13] ALSA: dice: use cache of stream formats to generate PCM rules/constraints Takashi Sakamoto
@ 2018-04-29  6:50 ` Takashi Sakamoto
  2018-04-29  6:50 ` [PATCH v2 02/13] ALSA: dice: add 'firewire' directory for proc nodes Takashi Sakamoto
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 21+ messages in thread
From: Takashi Sakamoto @ 2018-04-29  6:50 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel, ffado-devel

A previous commit 6f688268b3f4 ('ALSA: dice: purge generating channel
cache') purged cache of stream formats. DICE interface originally has
no feature to assist drivers to retrieve available formats for all of
supported sampling transmission frequencies, without changing the
frequency actually.

For later release of Dice ASICs such as TCD2210, Dice interface has
extended protocol and can support the feature. This assists drivers
to retrieve available stream formats.

This commit is a first step to regain the cache to generate PCM rules
for all of supported sampling transmission frequencies.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/dice/dice.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/sound/firewire/dice/dice.h b/sound/firewire/dice/dice.h
index da00e75e09d4..8112927999a1 100644
--- a/sound/firewire/dice/dice.h
+++ b/sound/firewire/dice/dice.h
@@ -80,6 +80,10 @@ struct snd_dice {
 	unsigned int rsrv_offset;
 
 	unsigned int clock_caps;
+	unsigned int tx_pcm_chs[MAX_STREAMS][3];
+	unsigned int rx_pcm_chs[MAX_STREAMS][3];
+	unsigned int tx_midi_ports[MAX_STREAMS];
+	unsigned int rx_midi_ports[MAX_STREAMS];
 
 	struct fw_address_handler notification_handler;
 	int owner_generation;
-- 
2.14.1

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

* [PATCH v2 02/13] ALSA: dice: add 'firewire' directory for proc nodes
  2018-04-29  6:50 [PATCH v2 00/13] ALSA: dice: use cache of stream formats to generate PCM rules/constraints Takashi Sakamoto
  2018-04-29  6:50 ` [PATCH v2 01/13] ALSA: dice: add cache of stream formats Takashi Sakamoto
@ 2018-04-29  6:50 ` Takashi Sakamoto
  2018-04-29  6:50 ` [PATCH v2 03/13] ALSA: dice: add proc node for stream formation Takashi Sakamoto
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 21+ messages in thread
From: Takashi Sakamoto @ 2018-04-29  6:50 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel, ffado-devel

Unlike the other drivers in ALSA firewire stack, ALSA dice driver does't
create 'firewire' directory for proc nodes because it has 'dice' node
only. But this is inconvenient because I have a plan to add another proc
node to output available stream formats from cache.

This commit let the driver to create the directory and put 'dice' node
into it.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/dice/dice-proc.c | 35 ++++++++++++++++++++++++++++++++---
 1 file changed, 32 insertions(+), 3 deletions(-)

diff --git a/sound/firewire/dice/dice-proc.c b/sound/firewire/dice/dice-proc.c
index cc079323ed30..43b130b7aa07 100644
--- a/sound/firewire/dice/dice-proc.c
+++ b/sound/firewire/dice/dice-proc.c
@@ -243,10 +243,39 @@ static void dice_proc_read(struct snd_info_entry *entry,
 	}
 }
 
-void snd_dice_create_proc(struct snd_dice *dice)
+static void add_node(struct snd_dice *dice, struct snd_info_entry *root,
+		     const char *name,
+		     void (*op)(struct snd_info_entry *entry,
+				struct snd_info_buffer *buffer))
 {
 	struct snd_info_entry *entry;
 
-	if (!snd_card_proc_new(dice->card, "dice", &entry))
-		snd_info_set_text_ops(entry, dice, dice_proc_read);
+	entry = snd_info_create_card_entry(dice->card, name, root);
+	if (!entry)
+		return;
+
+	snd_info_set_text_ops(entry, dice, op);
+	if (snd_info_register(entry) < 0)
+		snd_info_free_entry(entry);
+}
+
+void snd_dice_create_proc(struct snd_dice *dice)
+{
+	struct snd_info_entry *root;
+
+	/*
+	 * All nodes are automatically removed at snd_card_disconnect(),
+	 * by following to link list.
+	 */
+	root = snd_info_create_card_entry(dice->card, "firewire",
+					  dice->card->proc_root);
+	if (!root)
+		return;
+	root->mode = S_IFDIR | S_IRUGO | S_IXUGO;
+	if (snd_info_register(root) < 0) {
+		snd_info_free_entry(root);
+		return;
+	}
+
+	add_node(dice, root, "dice", dice_proc_read);
 }
-- 
2.14.1

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

* [PATCH v2 03/13] ALSA: dice: add proc node for stream formation
  2018-04-29  6:50 [PATCH v2 00/13] ALSA: dice: use cache of stream formats to generate PCM rules/constraints Takashi Sakamoto
  2018-04-29  6:50 ` [PATCH v2 01/13] ALSA: dice: add cache of stream formats Takashi Sakamoto
  2018-04-29  6:50 ` [PATCH v2 02/13] ALSA: dice: add 'firewire' directory for proc nodes Takashi Sakamoto
@ 2018-04-29  6:50 ` Takashi Sakamoto
  2018-04-30 15:12   ` Takashi Iwai
  2018-04-29  6:50 ` [PATCH v2 04/13] ALSA: dice: cache stream formats at current mode of sampling transmission frequency Takashi Sakamoto
                   ` (9 subsequent siblings)
  12 siblings, 1 reply; 21+ messages in thread
From: Takashi Sakamoto @ 2018-04-29  6:50 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel, ffado-devel

Products with DICE interface in market can support variable stream
formats for three levels of sampling transmission frequencies. To
record these formats, a proxy structure got several fields in former
commit.

This commit adds a proc node to output the stream formats for debugging
purpose.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/dice/dice-proc.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/sound/firewire/dice/dice-proc.c b/sound/firewire/dice/dice-proc.c
index 43b130b7aa07..f967e0ec7598 100644
--- a/sound/firewire/dice/dice-proc.c
+++ b/sound/firewire/dice/dice-proc.c
@@ -243,6 +243,40 @@ static void dice_proc_read(struct snd_info_entry *entry,
 	}
 }
 
+static void dice_proc_read_formation(struct snd_info_entry *entry,
+				     struct snd_info_buffer *buffer)
+{
+	static const char *const rate_labels[] = {
+		[0] = "low",
+		[1] = "middle",
+		[2] = "high",
+	};
+	struct snd_dice *dice = entry->private_data;
+	int i, j;
+
+	snd_iprintf(buffer, "Output stream from unit:\n");
+	for (i = 0; i < 3; ++i)
+		snd_iprintf(buffer, "\t%s", rate_labels[i]);
+	snd_iprintf(buffer, "\tMIDI\n");
+	for (i = 0; i < MAX_STREAMS; ++i) {
+		snd_iprintf(buffer, "Tx %u:", i);
+		for (j = 0; j < 3; ++j)
+			snd_iprintf(buffer, "\t%u", dice->tx_pcm_chs[i][j]);
+		snd_iprintf(buffer, "\t%u\n", dice->tx_midi_ports[i]);
+	}
+
+	snd_iprintf(buffer, "Input stream to unit:\n");
+	for (i = 0; i < 3; ++i)
+		snd_iprintf(buffer, "\t%s", rate_labels[i]);
+	snd_iprintf(buffer, "\n");
+	for (i = 0; i < MAX_STREAMS; ++i) {
+		snd_iprintf(buffer, "Rx %u:", i);
+		for (j = 0; j < 3; ++j)
+			snd_iprintf(buffer, "\t%u", dice->rx_pcm_chs[i][j]);
+		snd_iprintf(buffer, "\t%u\n", dice->rx_midi_ports[i]);
+	}
+}
+
 static void add_node(struct snd_dice *dice, struct snd_info_entry *root,
 		     const char *name,
 		     void (*op)(struct snd_info_entry *entry,
@@ -278,4 +312,5 @@ void snd_dice_create_proc(struct snd_dice *dice)
 	}
 
 	add_node(dice, root, "dice", dice_proc_read);
+	add_node(dice, root, "formation", dice_proc_read_formation);
 }
-- 
2.14.1

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

* [PATCH v2 04/13] ALSA: dice: cache stream formats at current mode of sampling transmission frequency
  2018-04-29  6:50 [PATCH v2 00/13] ALSA: dice: use cache of stream formats to generate PCM rules/constraints Takashi Sakamoto
                   ` (2 preceding siblings ...)
  2018-04-29  6:50 ` [PATCH v2 03/13] ALSA: dice: add proc node for stream formation Takashi Sakamoto
@ 2018-04-29  6:50 ` Takashi Sakamoto
  2018-04-30 15:10   ` Takashi Iwai
  2018-04-29  6:50 ` [PATCH v2 05/13] ALSA: dice: add parameters of stream formats for models produced by TC Electronic Takashi Sakamoto
                   ` (8 subsequent siblings)
  12 siblings, 1 reply; 21+ messages in thread
From: Takashi Sakamoto @ 2018-04-29  6:50 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel, ffado-devel

In former commits, proxy structure get members for cache of stream
formats. This commit fills the cache with stream formats at current mode
of sampling transmission frequency.

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

diff --git a/sound/firewire/dice/dice-stream.c b/sound/firewire/dice/dice-stream.c
index 928a255bfc35..2a9f0cd994a5 100644
--- a/sound/firewire/dice/dice-stream.c
+++ b/sound/firewire/dice/dice-stream.c
@@ -30,6 +30,24 @@ const unsigned int snd_dice_rates[SND_DICE_RATES_COUNT] = {
 	[6] = 192000,
 };
 
+int snd_dice_stream_get_rate_mode(struct snd_dice *dice, unsigned int rate,
+				  unsigned int *mode)
+{
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(snd_dice_rates); i++) {
+		if (!(dice->clock_caps & BIT(i)))
+			continue;
+		if (snd_dice_rates[i] != rate)
+			continue;
+
+		*mode = (i - 1) / 2;
+		return 0;
+	}
+
+	return -EINVAL;
+}
+
 /*
  * This operation has an effect to synchronize GLOBAL_STATUS/GLOBAL_SAMPLE_RATE
  * to GLOBAL_STATUS. Especially, just after powering on, these are different.
@@ -484,6 +502,64 @@ void snd_dice_stream_update_duplex(struct snd_dice *dice)
 	}
 }
 
+int snd_dice_stream_detect_current_formats(struct snd_dice *dice)
+{
+	unsigned int rate;
+	unsigned int mode;
+	__be32 reg[2];
+	struct reg_params tx_params, rx_params;
+	int i;
+	int err;
+
+	/*
+	 * Available stream format is restricted at current mode of sampling
+	 * clock.
+	 */
+	err = snd_dice_transaction_get_rate(dice, &rate);
+	if (err < 0)
+		return err;
+
+	err = snd_dice_stream_get_rate_mode(dice, rate, &mode);
+	if (err < 0)
+		return err;
+
+	/*
+	 * Just after owning the unit (GLOBAL_OWNER), the unit can return
+	 * invalid stream formats. Selecting clock parameters have an effect
+	 * for the unit to refine it.
+	 */
+	err = ensure_phase_lock(dice);
+	if (err < 0)
+		return err;
+
+	err = get_register_params(dice, &tx_params, &rx_params);
+	if (err < 0)
+		return err;
+
+	for (i = 0; i < tx_params.count; ++i) {
+		err = snd_dice_transaction_read_tx(dice,
+				tx_params.size * i + TX_NUMBER_AUDIO,
+				reg, sizeof(reg));
+		if (err < 0)
+			return err;
+		dice->tx_pcm_chs[i][mode] = be32_to_cpu(reg[0]);
+		dice->tx_midi_ports[i] = max_t(unsigned int,
+				be32_to_cpu(reg[1]), dice->tx_midi_ports[i]);
+	}
+	for (i = 0; i < rx_params.count; ++i) {
+		err = snd_dice_transaction_read_rx(dice,
+				rx_params.size * i + RX_NUMBER_AUDIO,
+				reg, sizeof(reg));
+		if (err < 0)
+			return err;
+		dice->rx_pcm_chs[i][mode] = be32_to_cpu(reg[0]);
+		dice->rx_midi_ports[i] = max_t(unsigned int,
+				be32_to_cpu(reg[1]), dice->rx_midi_ports[i]);
+	}
+
+	return 0;
+}
+
 static void dice_lock_changed(struct snd_dice *dice)
 {
 	dice->dev_lock_changed = true;
diff --git a/sound/firewire/dice/dice.c b/sound/firewire/dice/dice.c
index 96bb01b6b751..002f3f3cbc6a 100644
--- a/sound/firewire/dice/dice.c
+++ b/sound/firewire/dice/dice.c
@@ -199,6 +199,10 @@ static void do_registration(struct work_struct *work)
 
 	dice_card_strings(dice);
 
+	err = snd_dice_stream_detect_current_formats(dice);
+	if (err < 0)
+		goto error;
+
 	err = snd_dice_stream_init_duplex(dice);
 	if (err < 0)
 		goto error;
diff --git a/sound/firewire/dice/dice.h b/sound/firewire/dice/dice.h
index 8112927999a1..593d998a661a 100644
--- a/sound/firewire/dice/dice.h
+++ b/sound/firewire/dice/dice.h
@@ -194,11 +194,14 @@ void snd_dice_transaction_destroy(struct snd_dice *dice);
 #define SND_DICE_RATES_COUNT	7
 extern const unsigned int snd_dice_rates[SND_DICE_RATES_COUNT];
 
+int snd_dice_stream_get_rate_mode(struct snd_dice *dice, unsigned int rate,
+				  unsigned int *mode);
 int snd_dice_stream_start_duplex(struct snd_dice *dice, unsigned int rate);
 void snd_dice_stream_stop_duplex(struct snd_dice *dice);
 int snd_dice_stream_init_duplex(struct snd_dice *dice);
 void snd_dice_stream_destroy_duplex(struct snd_dice *dice);
 void snd_dice_stream_update_duplex(struct snd_dice *dice);
+int snd_dice_stream_detect_current_formats(struct snd_dice *dice);
 
 int snd_dice_stream_lock_try(struct snd_dice *dice);
 void snd_dice_stream_lock_release(struct snd_dice *dice);
-- 
2.14.1

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

* [PATCH v2 05/13] ALSA: dice: add parameters of stream formats for models produced by TC Electronic
  2018-04-29  6:50 [PATCH v2 00/13] ALSA: dice: use cache of stream formats to generate PCM rules/constraints Takashi Sakamoto
                   ` (3 preceding siblings ...)
  2018-04-29  6:50 ` [PATCH v2 04/13] ALSA: dice: cache stream formats at current mode of sampling transmission frequency Takashi Sakamoto
@ 2018-04-29  6:50 ` Takashi Sakamoto
  2018-04-29  6:50 ` [PATCH v2 06/13] ALSA: dice: add parameters of stream formats for models produced by Alesis Takashi Sakamoto
                   ` (7 subsequent siblings)
  12 siblings, 0 replies; 21+ messages in thread
From: Takashi Sakamoto @ 2018-04-29  6:50 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel, ffado-devel

TC Electronic shipped some models with DICE ASICs. All of them just support
DICE original protocol and drivers can't retrieve all of available stream
formats without changing status of sampling transmission frequency
actually.

This commit puts some hard-coded parameters for the models. When detecting
the models, the corresponding parameters are copied as cache of stream
formats.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/dice/Makefile            |   2 +-
 sound/firewire/dice/dice-tcelectronic.c | 100 ++++++++++++++++++++++++++++++++
 sound/firewire/dice/dice.c              |  76 +++++++++++++++++++++---
 sound/firewire/dice/dice.h              |   6 ++
 4 files changed, 174 insertions(+), 10 deletions(-)
 create mode 100644 sound/firewire/dice/dice-tcelectronic.c

diff --git a/sound/firewire/dice/Makefile b/sound/firewire/dice/Makefile
index 55b4be9b0034..5ffaa366a88c 100644
--- a/sound/firewire/dice/Makefile
+++ b/sound/firewire/dice/Makefile
@@ -1,3 +1,3 @@
 snd-dice-objs := dice-transaction.o dice-stream.o dice-proc.o dice-midi.o \
-		 dice-pcm.o dice-hwdep.o dice.o
+		 dice-pcm.o dice-hwdep.o dice.o dice-tcelectronic.o
 obj-$(CONFIG_SND_DICE) += snd-dice.o
diff --git a/sound/firewire/dice/dice-tcelectronic.c b/sound/firewire/dice/dice-tcelectronic.c
new file mode 100644
index 000000000000..d40542ff8a42
--- /dev/null
+++ b/sound/firewire/dice/dice-tcelectronic.c
@@ -0,0 +1,100 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * dice-tc_electronic.c - a part of driver for DICE based devices
+ *
+ * Copyright (c) 2018 Takashi Sakamoto
+ */
+
+#include "dice.h"
+
+struct dice_tc_spec {
+	unsigned int tx_pcm_chs[MAX_STREAMS][3];
+	unsigned int rx_pcm_chs[MAX_STREAMS][3];
+	bool has_midi;
+};
+
+static const struct dice_tc_spec desktop_konnekt6 = {
+	.tx_pcm_chs = {{6, 6, 2}, {0, 0, 0} },
+	.rx_pcm_chs = {{6, 6, 4}, {0, 0, 0} },
+	.has_midi = false,
+};
+
+static const struct dice_tc_spec impact_twin = {
+	.tx_pcm_chs = {{14, 10, 6}, {0, 0, 0} },
+	.rx_pcm_chs = {{14, 10, 6}, {0, 0, 0} },
+	.has_midi = true,
+};
+
+static const struct dice_tc_spec konnekt_8 = {
+	.tx_pcm_chs = {{4, 4, 3}, {0, 0, 0} },
+	.rx_pcm_chs = {{4, 4, 3}, {0, 0, 0} },
+	.has_midi = true,
+};
+
+static const struct dice_tc_spec konnekt_24d = {
+	.tx_pcm_chs = {{16, 16, 6}, {0, 0, 0} },
+	.rx_pcm_chs = {{16, 16, 6}, {0, 0, 0} },
+	.has_midi = true,
+};
+
+static const struct dice_tc_spec konnekt_live = {
+	.tx_pcm_chs = {{16, 16, 16}, {0, 0, 0} },
+	.rx_pcm_chs = {{16, 16, 16}, {0, 0, 0} },
+	.has_midi = true,
+};
+
+static const struct dice_tc_spec studio_konnekt_48 = {
+	.tx_pcm_chs = {{16, 16, 16}, {16, 16, 0} },
+	.rx_pcm_chs = {{16, 16, 16}, {16, 16, 0} },
+	.has_midi = true,
+};
+
+int snd_dice_detect_tcelectronic_formats(struct snd_dice *dice)
+{
+	static const struct {
+		u32 model_id;
+		const struct dice_tc_spec *spec;
+	} *entry, entries[] = {
+		{0x00000020, &konnekt_24d},
+		{0x00000021, &konnekt_8},
+		{0x00000022, &studio_konnekt_48},
+		{0x00000023, &konnekt_live},
+		{0x00000024, &desktop_konnekt6},
+		{0x00000027, &impact_twin},
+	};
+	struct fw_csr_iterator it;
+	int key, val, model_id;
+	int i;
+
+	model_id = 0;
+	fw_csr_iterator_init(&it, dice->unit->directory);
+	while (fw_csr_iterator_next(&it, &key, &val)) {
+		if (key == CSR_MODEL) {
+			model_id = val;
+			break;
+		}
+	}
+
+	entry = NULL;
+	for (i = 0; i < ARRAY_SIZE(entries); ++i) {
+		entry = entries + i;
+		if (entry->model_id == model_id)
+			break;
+	}
+	if (!entry)
+		return -ENODEV;
+
+	memcpy(dice->tx_pcm_chs, entry->spec->tx_pcm_chs,
+	       MAX_STREAMS * 3 * sizeof(unsigned int));
+	memcpy(dice->rx_pcm_chs, entry->spec->rx_pcm_chs,
+	       MAX_STREAMS * 3 * sizeof(unsigned int));
+
+	for (i = 0; i < MAX_STREAMS; ++i) {
+		if (entry->spec->has_midi) {
+			dice->tx_midi_ports[i] = 1;
+			dice->rx_midi_ports[i] = 1;
+		}
+	}
+
+	return 0;
+}
diff --git a/sound/firewire/dice/dice.c b/sound/firewire/dice/dice.c
index 002f3f3cbc6a..ea112506cc66 100644
--- a/sound/firewire/dice/dice.c
+++ b/sound/firewire/dice/dice.c
@@ -199,7 +199,7 @@ static void do_registration(struct work_struct *work)
 
 	dice_card_strings(dice);
 
-	err = snd_dice_stream_detect_current_formats(dice);
+	err = dice->detect_formats(dice);
 	if (err < 0)
 		goto error;
 
@@ -243,14 +243,17 @@ static void do_registration(struct work_struct *work)
 		 "Sound card registration failed: %d\n", err);
 }
 
-static int dice_probe(struct fw_unit *unit, const struct ieee1394_device_id *id)
+static int dice_probe(struct fw_unit *unit,
+		      const struct ieee1394_device_id *entry)
 {
 	struct snd_dice *dice;
 	int err;
 
-	err = check_dice_category(unit);
-	if (err < 0)
-		return -ENODEV;
+	if (!entry->driver_data) {
+		err = check_dice_category(unit);
+		if (err < 0)
+			return -ENODEV;
+	}
 
 	/* Allocate this independent of sound card instance. */
 	dice = kzalloc(sizeof(struct snd_dice), GFP_KERNEL);
@@ -260,6 +263,13 @@ static int dice_probe(struct fw_unit *unit, const struct ieee1394_device_id *id)
 	dice->unit = fw_unit_get(unit);
 	dev_set_drvdata(&unit->device, dice);
 
+	if (!entry->driver_data) {
+		dice->detect_formats = snd_dice_stream_detect_current_formats;
+	} else {
+		dice->detect_formats =
+				(snd_dice_detect_formats_t)entry->driver_data;
+	}
+
 	spin_lock_init(&dice->lock);
 	mutex_init(&dice->mutex);
 	init_completion(&dice->clock_accepted);
@@ -317,10 +327,6 @@ static void dice_bus_reset(struct fw_unit *unit)
 #define DICE_INTERFACE	0x000001
 
 static const struct ieee1394_device_id dice_id_table[] = {
-	{
-		.match_flags = IEEE1394_MATCH_VERSION,
-		.version     = DICE_INTERFACE,
-	},
 	/* M-Audio Profire 610/2626 has a different value in version field. */
 	{
 		.match_flags	= IEEE1394_MATCH_VENDOR_ID |
@@ -328,6 +334,58 @@ static const struct ieee1394_device_id dice_id_table[] = {
 		.vendor_id	= 0x000d6c,
 		.specifier_id	= 0x000d6c,
 	},
+	/* TC Electronic Konnekt 24D. */
+	{
+		.match_flags	= IEEE1394_MATCH_VENDOR_ID |
+				  IEEE1394_MATCH_MODEL_ID,
+		.vendor_id	= OUI_TCELECTRONIC,
+		.model_id	= 0x000020,
+		.driver_data = (kernel_ulong_t)snd_dice_detect_tcelectronic_formats,
+	},
+	/* TC Electronic Konnekt 8. */
+	{
+		.match_flags	= IEEE1394_MATCH_VENDOR_ID |
+				  IEEE1394_MATCH_MODEL_ID,
+		.vendor_id	= OUI_TCELECTRONIC,
+		.model_id	= 0x000021,
+		.driver_data = (kernel_ulong_t)snd_dice_detect_tcelectronic_formats,
+	},
+	/* TC Electronic Studio Konnekt 48. */
+	{
+		.match_flags	= IEEE1394_MATCH_VENDOR_ID |
+				  IEEE1394_MATCH_MODEL_ID,
+		.vendor_id	= OUI_TCELECTRONIC,
+		.model_id	= 0x000022,
+		.driver_data = (kernel_ulong_t)snd_dice_detect_tcelectronic_formats,
+	},
+	/* TC Electronic Konnekt Live. */
+	{
+		.match_flags	= IEEE1394_MATCH_VENDOR_ID |
+				  IEEE1394_MATCH_MODEL_ID,
+		.vendor_id	= OUI_TCELECTRONIC,
+		.model_id	= 0x000023,
+		.driver_data = (kernel_ulong_t)snd_dice_detect_tcelectronic_formats,
+	},
+	/* TC Electronic Desktop Konnekt 6. */
+	{
+		.match_flags	= IEEE1394_MATCH_VENDOR_ID |
+				  IEEE1394_MATCH_MODEL_ID,
+		.vendor_id	= OUI_TCELECTRONIC,
+		.model_id	= 0x000024,
+		.driver_data = (kernel_ulong_t)snd_dice_detect_tcelectronic_formats,
+	},
+	/* TC Electronic Impact Twin. */
+	{
+		.match_flags	= IEEE1394_MATCH_VENDOR_ID |
+				  IEEE1394_MATCH_MODEL_ID,
+		.vendor_id	= OUI_TCELECTRONIC,
+		.model_id	= 0x000027,
+		.driver_data = (kernel_ulong_t)snd_dice_detect_tcelectronic_formats,
+	},
+	{
+		.match_flags = IEEE1394_MATCH_VERSION,
+		.version     = DICE_INTERFACE,
+	},
 	{ }
 };
 MODULE_DEVICE_TABLE(ieee1394, dice_id_table);
diff --git a/sound/firewire/dice/dice.h b/sound/firewire/dice/dice.h
index 593d998a661a..c31a4ebc5b51 100644
--- a/sound/firewire/dice/dice.h
+++ b/sound/firewire/dice/dice.h
@@ -63,6 +63,9 @@
  */
 #define MAX_STREAMS	2
 
+struct snd_dice;
+typedef int (*snd_dice_detect_formats_t)(struct snd_dice *dice);
+
 struct snd_dice {
 	struct snd_card *card;
 	struct fw_unit *unit;
@@ -84,6 +87,7 @@ struct snd_dice {
 	unsigned int rx_pcm_chs[MAX_STREAMS][3];
 	unsigned int tx_midi_ports[MAX_STREAMS];
 	unsigned int rx_midi_ports[MAX_STREAMS];
+	snd_dice_detect_formats_t detect_formats;
 
 	struct fw_address_handler notification_handler;
 	int owner_generation;
@@ -214,4 +218,6 @@ void snd_dice_create_proc(struct snd_dice *dice);
 
 int snd_dice_create_midi(struct snd_dice *dice);
 
+int snd_dice_detect_tcelectronic_formats(struct snd_dice *dice);
+
 #endif
-- 
2.14.1

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

* [PATCH v2 06/13] ALSA: dice: add parameters of stream formats for models produced by Alesis
  2018-04-29  6:50 [PATCH v2 00/13] ALSA: dice: use cache of stream formats to generate PCM rules/constraints Takashi Sakamoto
                   ` (4 preceding siblings ...)
  2018-04-29  6:50 ` [PATCH v2 05/13] ALSA: dice: add parameters of stream formats for models produced by TC Electronic Takashi Sakamoto
@ 2018-04-29  6:50 ` Takashi Sakamoto
  2018-04-29  6:50 ` [PATCH v2 07/13] ALSA: dice: use extended protocol to detect available stream formats Takashi Sakamoto
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 21+ messages in thread
From: Takashi Sakamoto @ 2018-04-29  6:50 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel, ffado-devel

Alesis shipped some models with DICE ASICs. All of them just support
DICE original protocol and drivers can't retrieve all of available stream
formats without changing status of sampling transmission frequency
actually.

This commit puts some hard-coded parameters for the models. When detecting
the models, the corresponding parameters are copied as cache of stream
formats.

I note that each of pair of iO14/iO26 and MultiMix 8/12/16 has the same
model ID on their configuration ROM. The MultiMix 8/12/16 just support
one mode for sampling transmission frequency and ALSA dice driver already
handles them correctly. The iO14/iO26 support three modes and need
hard-coded parameters. To distinguish these two models, this commit let
the driver to retrieve current stream formats and compare it to known
parameters, then decide it.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/dice/Makefile      |  3 ++-
 sound/firewire/dice/dice-alesis.c | 48 +++++++++++++++++++++++++++++++++++++++
 sound/firewire/dice/dice.c        | 11 +++++++++
 sound/firewire/dice/dice.h        |  1 +
 4 files changed, 62 insertions(+), 1 deletion(-)
 create mode 100644 sound/firewire/dice/dice-alesis.c

diff --git a/sound/firewire/dice/Makefile b/sound/firewire/dice/Makefile
index 5ffaa366a88c..5cfe618f45ef 100644
--- a/sound/firewire/dice/Makefile
+++ b/sound/firewire/dice/Makefile
@@ -1,3 +1,4 @@
 snd-dice-objs := dice-transaction.o dice-stream.o dice-proc.o dice-midi.o \
-		 dice-pcm.o dice-hwdep.o dice.o dice-tcelectronic.o
+		 dice-pcm.o dice-hwdep.o dice.o dice-tcelectronic.o \
+		 dice-alesis.o
 obj-$(CONFIG_SND_DICE) += snd-dice.o
diff --git a/sound/firewire/dice/dice-alesis.c b/sound/firewire/dice/dice-alesis.c
new file mode 100644
index 000000000000..2dd554653618
--- /dev/null
+++ b/sound/firewire/dice/dice-alesis.c
@@ -0,0 +1,48 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * dice-alesis.c - a part of driver for DICE based devices
+ *
+ * Copyright (c) 2018 Takashi Sakamoto
+ */
+
+#include "dice.h"
+
+static const unsigned int alesis_io14_tx_pcm_chs[MAX_STREAMS][3] = {
+	{6, 6, 4},	/* Tx0 = Analog + S/PDIF. */
+	{8, 4, 0},	/* Tx1 = ADAT1. */
+};
+
+static const unsigned int alesis_io26_tx_pcm_chs[MAX_STREAMS][3] = {
+	{10, 10, 8},	/* Tx0 = Analog + S/PDIF. */
+	{16, 8, 0},	/* Tx1 = ADAT1 + ADAT2. */
+};
+
+int snd_dice_detect_alesis_formats(struct snd_dice *dice)
+{
+	__be32 reg;
+	u32 data;
+	int i;
+	int err;
+
+	err = snd_dice_transaction_read_tx(dice, TX_NUMBER_AUDIO, &reg,
+					   sizeof(reg));
+	if (err < 0)
+		return err;
+	data = be32_to_cpu(reg);
+
+	if (data == 4 || data == 6) {
+		memcpy(dice->tx_pcm_chs, alesis_io14_tx_pcm_chs,
+		       MAX_STREAMS * 3 * sizeof(unsigned int));
+	} else {
+		memcpy(dice->rx_pcm_chs, alesis_io26_tx_pcm_chs,
+		       MAX_STREAMS * 3 * sizeof(unsigned int));
+	}
+
+	for (i = 0; i < 3; ++i)
+		dice->rx_pcm_chs[0][i] = 8;
+
+	dice->tx_midi_ports[0] = 1;
+	dice->rx_midi_ports[0] = 1;
+
+	return 0;
+}
diff --git a/sound/firewire/dice/dice.c b/sound/firewire/dice/dice.c
index ea112506cc66..cbd1a07e70b9 100644
--- a/sound/firewire/dice/dice.c
+++ b/sound/firewire/dice/dice.c
@@ -15,11 +15,14 @@ MODULE_LICENSE("GPL v2");
 #define OUI_LOUD		0x000ff2
 #define OUI_FOCUSRITE		0x00130e
 #define OUI_TCELECTRONIC	0x000166
+#define OUI_ALESIS		0x000595
 
 #define DICE_CATEGORY_ID	0x04
 #define WEISS_CATEGORY_ID	0x00
 #define LOUD_CATEGORY_ID	0x10
 
+#define MODEL_ALESIS_IO_BOTH	0x000001
+
 /*
  * Some models support several isochronous channels, while these streams are not
  * always available. In this case, add the model name to this list.
@@ -382,6 +385,14 @@ static const struct ieee1394_device_id dice_id_table[] = {
 		.model_id	= 0x000027,
 		.driver_data = (kernel_ulong_t)snd_dice_detect_tcelectronic_formats,
 	},
+	/* Alesis iO14/iO26. */
+	{
+		.match_flags	= IEEE1394_MATCH_VENDOR_ID |
+				  IEEE1394_MATCH_MODEL_ID,
+		.vendor_id	= OUI_ALESIS,
+		.model_id	= MODEL_ALESIS_IO_BOTH,
+		.driver_data = (kernel_ulong_t)snd_dice_detect_alesis_formats,
+	},
 	{
 		.match_flags = IEEE1394_MATCH_VERSION,
 		.version     = DICE_INTERFACE,
diff --git a/sound/firewire/dice/dice.h b/sound/firewire/dice/dice.h
index c31a4ebc5b51..bb89dad5f127 100644
--- a/sound/firewire/dice/dice.h
+++ b/sound/firewire/dice/dice.h
@@ -219,5 +219,6 @@ void snd_dice_create_proc(struct snd_dice *dice);
 int snd_dice_create_midi(struct snd_dice *dice);
 
 int snd_dice_detect_tcelectronic_formats(struct snd_dice *dice);
+int snd_dice_detect_alesis_formats(struct snd_dice *dice);
 
 #endif
-- 
2.14.1

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

* [PATCH v2 07/13] ALSA: dice: use extended protocol to detect available stream formats
  2018-04-29  6:50 [PATCH v2 00/13] ALSA: dice: use cache of stream formats to generate PCM rules/constraints Takashi Sakamoto
                   ` (5 preceding siblings ...)
  2018-04-29  6:50 ` [PATCH v2 06/13] ALSA: dice: add parameters of stream formats for models produced by Alesis Takashi Sakamoto
@ 2018-04-29  6:50 ` Takashi Sakamoto
  2018-04-29  6:50 ` [PATCH v2 08/13] ALSA: dice: use cache of stream format to check running stream Takashi Sakamoto
                   ` (5 subsequent siblings)
  12 siblings, 0 replies; 21+ messages in thread
From: Takashi Sakamoto @ 2018-04-29  6:50 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel, ffado-devel

TC Applied Technologies (TCAT) have added extension to DICE protocol. This
protocol extension is called as Extended Application Protocol, a.k.a. EAP.

In this protocol extension, units get additional 9 address spaces. One of
it is for current configuration. In this address space, a pair of router
and stream formats are exposed per mode of three sampling transmission
frequencies.

This commit adds support the protocol extension for address space of the
current configuration to generate cache of stream formats.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/dice/Makefile         |   2 +-
 sound/firewire/dice/dice-extension.c | 172 +++++++++++++++++++++++++++++++++++
 sound/firewire/dice/dice-stream.c    |   5 +
 sound/firewire/dice/dice.c           |  18 +++-
 sound/firewire/dice/dice.h           |   1 +
 5 files changed, 193 insertions(+), 5 deletions(-)
 create mode 100644 sound/firewire/dice/dice-extension.c

diff --git a/sound/firewire/dice/Makefile b/sound/firewire/dice/Makefile
index 5cfe618f45ef..7b7997a5754c 100644
--- a/sound/firewire/dice/Makefile
+++ b/sound/firewire/dice/Makefile
@@ -1,4 +1,4 @@
 snd-dice-objs := dice-transaction.o dice-stream.o dice-proc.o dice-midi.o \
 		 dice-pcm.o dice-hwdep.o dice.o dice-tcelectronic.o \
-		 dice-alesis.o
+		 dice-alesis.o dice-extension.o
 obj-$(CONFIG_SND_DICE) += snd-dice.o
diff --git a/sound/firewire/dice/dice-extension.c b/sound/firewire/dice/dice-extension.c
new file mode 100644
index 000000000000..afda705cf722
--- /dev/null
+++ b/sound/firewire/dice/dice-extension.c
@@ -0,0 +1,172 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * dice-extension.c - a part of driver for DICE based devices
+ *
+ * Copyright (c) 2018 Takashi Sakamoto
+ */
+
+#include "dice.h"
+
+/* For TCD2210/2220, TCAT defines extension of application protocol. */
+
+#define DICE_EXT_APP_SPACE		0xffffe0200000uLL
+
+#define DICE_EXT_APP_CAPS_OFFSET	0x00
+#define DICE_EXT_APP_CAPS_SIZE		0x04
+#define DICE_EXT_APP_CMD_OFFSET		0x08
+#define DICE_EXT_APP_CMD_SIZE		0x0c
+#define DICE_EXT_APP_MIXER_OFFSET	0x10
+#define DICE_EXT_APP_MIXER_SIZE		0x14
+#define DICE_EXT_APP_PEAK_OFFSET	0x18
+#define DICE_EXT_APP_PEAK_SIZE		0x1c
+#define DICE_EXT_APP_ROUTER_OFFSET	0x20
+#define DICE_EXT_APP_ROUTER_SIZE	0x24
+#define DICE_EXT_APP_STREAM_OFFSET	0x28
+#define DICE_EXT_APP_STREAM_SIZE	0x2c
+#define DICE_EXT_APP_CURRENT_OFFSET	0x30
+#define DICE_EXT_APP_CURRENT_SIZE	0x34
+#define DICE_EXT_APP_STANDALONE_OFFSET	0x38
+#define DICE_EXT_APP_STANDALONE_SIZE	0x3c
+#define DICE_EXT_APP_APPLICATION_OFFSET	0x40
+#define DICE_EXT_APP_APPLICATION_SIZE	0x44
+
+#define EXT_APP_STREAM_TX_NUMBER	0x0000
+#define EXT_APP_STREAM_RX_NUMBER	0x0004
+#define EXT_APP_STREAM_ENTRIES		0x0008
+#define EXT_APP_STREAM_ENTRY_SIZE	0x010c
+#define  EXT_APP_NUMBER_AUDIO		0x0000
+#define  EXT_APP_NUMBER_MIDI		0x0004
+#define  EXT_APP_NAMES			0x0008
+#define   EXT_APP_NAMES_SIZE		256
+#define  EXT_APP_AC3			0x0108
+
+#define EXT_APP_CONFIG_LOW_ROUTER	0x0000
+#define EXT_APP_CONFIG_LOW_STREAM	0x1000
+#define EXT_APP_CONFIG_MIDDLE_ROUTER	0x2000
+#define EXT_APP_CONFIG_MIDDLE_STREAM	0x3000
+#define EXT_APP_CONFIG_HIGH_ROUTER	0x4000
+#define EXT_APP_CONFIG_HIGH_STREAM	0x5000
+
+static inline int read_transaction(struct snd_dice *dice, u64 section_addr,
+				   u32 offset, void *buf, size_t len)
+{
+	return snd_fw_transaction(dice->unit,
+				  len == 4 ? TCODE_READ_QUADLET_REQUEST :
+					     TCODE_READ_BLOCK_REQUEST,
+				  section_addr + offset, buf, len, 0);
+}
+
+static int read_stream_entries(struct snd_dice *dice, u64 section_addr,
+			       u32 base_offset, unsigned int stream_count,
+			       unsigned int mode,
+			       unsigned int pcm_channels[MAX_STREAMS][3],
+			       unsigned int midi_ports[MAX_STREAMS])
+{
+	u32 entry_offset;
+	__be32 reg[2];
+	int err;
+	int i;
+
+	for (i = 0; i < stream_count; ++i) {
+		entry_offset = base_offset + i * EXT_APP_STREAM_ENTRY_SIZE;
+		err = read_transaction(dice, section_addr,
+				    entry_offset + EXT_APP_NUMBER_AUDIO,
+				    reg, sizeof(reg));
+		if (err < 0)
+			return err;
+		pcm_channels[i][mode] = be32_to_cpu(reg[0]);
+		midi_ports[i] = max(midi_ports[i], be32_to_cpu(reg[1]));
+	}
+
+	return 0;
+}
+
+static int detect_stream_formats(struct snd_dice *dice, u64 section_addr)
+{
+	u32 base_offset;
+	__be32 reg[2];
+	unsigned int stream_count;
+	int mode;
+	int err = 0;
+
+	for (mode = 0; mode < 3; ++mode) {
+		unsigned int cap;
+
+		/*
+		 * Some models report stream formats at highest mode, however
+		 * they don't support the mode. Check clock capabilities.
+		 */
+		if (mode == 2) {
+			cap = CLOCK_CAP_RATE_176400 | CLOCK_CAP_RATE_192000;
+		} else if (mode == 1) {
+			cap = CLOCK_CAP_RATE_88200 | CLOCK_CAP_RATE_96000;
+		} else {
+			cap = CLOCK_CAP_RATE_32000 | CLOCK_CAP_RATE_44100 |
+			      CLOCK_CAP_RATE_48000;
+		}
+		if (!(cap & dice->clock_caps))
+			continue;
+
+		base_offset = 0x2000 * mode + 0x1000;
+
+		err = read_transaction(dice, section_addr,
+				       base_offset + EXT_APP_STREAM_TX_NUMBER,
+				       &reg, sizeof(reg));
+		if (err < 0)
+			break;
+
+		base_offset += EXT_APP_STREAM_ENTRIES;
+		stream_count = be32_to_cpu(reg[0]);
+		err = read_stream_entries(dice, section_addr, base_offset,
+					  stream_count, mode,
+					  dice->tx_pcm_chs,
+					  dice->tx_midi_ports);
+		if (err < 0)
+			break;
+
+		base_offset += stream_count * EXT_APP_STREAM_ENTRY_SIZE;
+		stream_count = be32_to_cpu(reg[1]);
+		err = read_stream_entries(dice, section_addr, base_offset,
+					  stream_count,
+					  mode, dice->rx_pcm_chs,
+					  dice->rx_midi_ports);
+		if (err < 0)
+			break;
+	}
+
+	return err;
+}
+
+int snd_dice_detect_extension_formats(struct snd_dice *dice)
+{
+	__be32 *pointers;
+	unsigned int i;
+	u64 section_addr;
+	int err;
+
+	pointers = kmalloc_array(9, sizeof(__be32) * 2, GFP_KERNEL);
+	if (pointers == NULL)
+		return -ENOMEM;
+
+	err = snd_fw_transaction(dice->unit, TCODE_READ_BLOCK_REQUEST,
+				 DICE_EXT_APP_SPACE, pointers,
+				 9 * sizeof(__be32) * 2, 0);
+	if (err < 0)
+		goto end;
+
+	/* Check two of them for offset have the same value or not. */
+	for (i = 0; i < 9; ++i) {
+		int j;
+
+		for (j = i + 1; j < 9; ++j) {
+			if (pointers[i * 2] == pointers[j * 2])
+				goto end;
+		}
+	}
+
+	section_addr = DICE_EXT_APP_SPACE + be32_to_cpu(pointers[12]) * 4;
+	err = detect_stream_formats(dice, section_addr);
+end:
+	kfree(pointers);
+	return err;
+}
diff --git a/sound/firewire/dice/dice-stream.c b/sound/firewire/dice/dice-stream.c
index 2a9f0cd994a5..da0607832456 100644
--- a/sound/firewire/dice/dice-stream.c
+++ b/sound/firewire/dice/dice-stream.c
@@ -511,6 +511,11 @@ int snd_dice_stream_detect_current_formats(struct snd_dice *dice)
 	int i;
 	int err;
 
+	/* If extended protocol is available, detect detail spec. */
+	err = snd_dice_detect_extension_formats(dice);
+	if (err >= 0)
+		return err;
+
 	/*
 	 * Available stream format is restricted at current mode of sampling
 	 * clock.
diff --git a/sound/firewire/dice/dice.c b/sound/firewire/dice/dice.c
index cbd1a07e70b9..6d55a62ec89e 100644
--- a/sound/firewire/dice/dice.c
+++ b/sound/firewire/dice/dice.c
@@ -16,6 +16,7 @@ MODULE_LICENSE("GPL v2");
 #define OUI_FOCUSRITE		0x00130e
 #define OUI_TCELECTRONIC	0x000166
 #define OUI_ALESIS		0x000595
+#define OUI_MAUDIO		0x000d6c
 
 #define DICE_CATEGORY_ID	0x04
 #define WEISS_CATEGORY_ID	0x00
@@ -330,12 +331,21 @@ static void dice_bus_reset(struct fw_unit *unit)
 #define DICE_INTERFACE	0x000001
 
 static const struct ieee1394_device_id dice_id_table[] = {
-	/* M-Audio Profire 610/2626 has a different value in version field. */
+	/* M-Audio Profire 2626 has a different value in version field. */
 	{
 		.match_flags	= IEEE1394_MATCH_VENDOR_ID |
-				  IEEE1394_MATCH_SPECIFIER_ID,
-		.vendor_id	= 0x000d6c,
-		.specifier_id	= 0x000d6c,
+				  IEEE1394_MATCH_MODEL_ID,
+		.vendor_id	= OUI_MAUDIO,
+		.model_id	= 0x000010,
+		.driver_data = (kernel_ulong_t)snd_dice_detect_extension_formats,
+	},
+	/* M-Audio Profire 610 has a different value in version field. */
+	{
+		.match_flags	= IEEE1394_MATCH_VENDOR_ID |
+				  IEEE1394_MATCH_MODEL_ID,
+		.vendor_id	= OUI_MAUDIO,
+		.model_id	= 0x000011,
+		.driver_data = (kernel_ulong_t)snd_dice_detect_extension_formats,
 	},
 	/* TC Electronic Konnekt 24D. */
 	{
diff --git a/sound/firewire/dice/dice.h b/sound/firewire/dice/dice.h
index bb89dad5f127..7b1e2396eadc 100644
--- a/sound/firewire/dice/dice.h
+++ b/sound/firewire/dice/dice.h
@@ -220,5 +220,6 @@ int snd_dice_create_midi(struct snd_dice *dice);
 
 int snd_dice_detect_tcelectronic_formats(struct snd_dice *dice);
 int snd_dice_detect_alesis_formats(struct snd_dice *dice);
+int snd_dice_detect_extension_formats(struct snd_dice *dice);
 
 #endif
-- 
2.14.1

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

* [PATCH v2 08/13] ALSA: dice: use cache of stream format to check running stream
  2018-04-29  6:50 [PATCH v2 00/13] ALSA: dice: use cache of stream formats to generate PCM rules/constraints Takashi Sakamoto
                   ` (6 preceding siblings ...)
  2018-04-29  6:50 ` [PATCH v2 07/13] ALSA: dice: use extended protocol to detect available stream formats Takashi Sakamoto
@ 2018-04-29  6:50 ` Takashi Sakamoto
  2018-04-29  6:50 ` [PATCH v2 09/13] ALSA: dice: add a helper function to restart all of available streams Takashi Sakamoto
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 21+ messages in thread
From: Takashi Sakamoto @ 2018-04-29  6:50 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel, ffado-devel

At present, to check running stream, available stream formats are used
at current sampling transmission frequency (stf). But when changing stf,
it's convenient to use cache of stream formats.

This commit applies this idea.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/dice/dice-stream.c | 50 ++++++++++++++++++++++++---------------
 1 file changed, 31 insertions(+), 19 deletions(-)

diff --git a/sound/firewire/dice/dice-stream.c b/sound/firewire/dice/dice-stream.c
index da0607832456..b81e98c238c7 100644
--- a/sound/firewire/dice/dice-stream.c
+++ b/sound/firewire/dice/dice-stream.c
@@ -284,16 +284,14 @@ int snd_dice_stream_start_duplex(struct snd_dice *dice, unsigned int rate)
 	unsigned int curr_rate;
 	unsigned int i;
 	struct reg_params tx_params, rx_params;
-	bool need_to_start;
+	bool need_to_start = false;
+	unsigned int mode;
 	int err;
 
 	if (dice->substreams_counter == 0)
 		return -EIO;
 
-	err = get_register_params(dice, &tx_params, &rx_params);
-	if (err < 0)
-		return err;
-
+	/* Check sampling transmission frequency. */
 	err = snd_dice_transaction_get_rate(dice, &curr_rate);
 	if (err < 0) {
 		dev_err(&dice->unit->device,
@@ -305,22 +303,36 @@ int snd_dice_stream_start_duplex(struct snd_dice *dice, unsigned int rate)
 	if (rate != curr_rate)
 		return -EINVAL;
 
-	/* Judge to need to restart streams. */
-	for (i = 0; i < MAX_STREAMS; i++) {
-		if (i < tx_params.count) {
-			if (amdtp_streaming_error(&dice->tx_stream[i]) ||
-			    !amdtp_stream_running(&dice->tx_stream[i]))
-				break;
-		}
-		if (i < rx_params.count) {
-			if (amdtp_streaming_error(&dice->rx_stream[i]) ||
-			    !amdtp_stream_running(&dice->rx_stream[i]))
-				break;
-		}
+	/* Check error of packet streaming. */
+	for (i = 0; i < MAX_STREAMS; ++i) {
+		if (amdtp_streaming_error(&dice->tx_stream[i]))
+			break;
+		if (amdtp_streaming_error(&dice->rx_stream[i]))
+			break;
+	}
+	if (i < MAX_STREAMS)
+		need_to_start = true;
+
+	/* Check required streams are running or not. */
+	err = snd_dice_stream_get_rate_mode(dice, rate, &mode);
+	if (err < 0)
+		return err;
+	for (i = 0; i < MAX_STREAMS; ++i) {
+		if (dice->tx_pcm_chs[i][mode] > 0 &&
+		    !amdtp_stream_running(&dice->tx_stream[i]))
+			break;
+		if (dice->rx_pcm_chs[i][mode] > 0 &&
+		    !amdtp_stream_running(&dice->rx_stream[i]))
+			break;
 	}
-	need_to_start = (i < MAX_STREAMS);
+	if (i < MAX_STREAMS)
+		need_to_start = true;
 
 	if (need_to_start) {
+		err = get_register_params(dice, &tx_params, &rx_params);
+		if (err < 0)
+			return err;
+
 		/* Stop transmission. */
 		snd_dice_transaction_clear_enable(dice);
 		stop_streams(dice, AMDTP_IN_STREAM, &tx_params);
@@ -331,7 +343,7 @@ int snd_dice_stream_start_duplex(struct snd_dice *dice, unsigned int rate)
 		if (err < 0) {
 			dev_err(&dice->unit->device,
 				"fail to ensure phase lock\n");
-			return err;
+			goto error;
 		}
 
 		/* Start both streams. */
-- 
2.14.1

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

* [PATCH v2 09/13] ALSA: dice: add a helper function to restart all of available streams
  2018-04-29  6:50 [PATCH v2 00/13] ALSA: dice: use cache of stream formats to generate PCM rules/constraints Takashi Sakamoto
                   ` (7 preceding siblings ...)
  2018-04-29  6:50 ` [PATCH v2 08/13] ALSA: dice: use cache of stream format to check running stream Takashi Sakamoto
@ 2018-04-29  6:50 ` Takashi Sakamoto
  2018-04-29  6:50 ` [PATCH v2 10/13] ALSA: dice: enable to change current sampling transmission frequency Takashi Sakamoto
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 21+ messages in thread
From: Takashi Sakamoto @ 2018-04-29  6:50 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel, ffado-devel

This commit is a small refactoring for better readability.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/dice/dice-stream.c | 119 ++++++++++++++++++++------------------
 1 file changed, 62 insertions(+), 57 deletions(-)

diff --git a/sound/firewire/dice/dice-stream.c b/sound/firewire/dice/dice-stream.c
index b81e98c238c7..6a73528013d3 100644
--- a/sound/firewire/dice/dice-stream.c
+++ b/sound/firewire/dice/dice-stream.c
@@ -274,6 +274,63 @@ static int start_streams(struct snd_dice *dice, enum amdtp_stream_direction dir,
 	return err;
 }
 
+static int start_duplex_streams(struct snd_dice *dice, unsigned int rate)
+{
+	struct reg_params tx_params, rx_params;
+	int i;
+	int err;
+
+	err = get_register_params(dice, &tx_params, &rx_params);
+	if (err < 0)
+		return err;
+
+	/* Stop transmission. */
+	stop_streams(dice, AMDTP_IN_STREAM, &tx_params);
+	stop_streams(dice, AMDTP_OUT_STREAM, &rx_params);
+	snd_dice_transaction_clear_enable(dice);
+	release_resources(dice);
+
+	err = ensure_phase_lock(dice);
+	if (err < 0) {
+		dev_err(&dice->unit->device, "fail to ensure phase lock\n");
+		return err;
+	}
+
+	/* Start both streams. */
+	err = start_streams(dice, AMDTP_IN_STREAM, rate, &tx_params);
+	if (err < 0)
+		goto error;
+	err = start_streams(dice, AMDTP_OUT_STREAM, rate, &rx_params);
+	if (err < 0)
+		goto error;
+
+	err = snd_dice_transaction_set_enable(dice);
+	if (err < 0) {
+		dev_err(&dice->unit->device, "fail to enable interface\n");
+		goto error;
+	}
+
+	for (i = 0; i < MAX_STREAMS; i++) {
+		if ((i < tx_params.count &&
+		    !amdtp_stream_wait_callback(&dice->tx_stream[i],
+						CALLBACK_TIMEOUT)) ||
+		    (i < rx_params.count &&
+		     !amdtp_stream_wait_callback(&dice->rx_stream[i],
+						 CALLBACK_TIMEOUT))) {
+			err = -ETIMEDOUT;
+			goto error;
+		}
+	}
+
+	return 0;
+error:
+	stop_streams(dice, AMDTP_IN_STREAM, &tx_params);
+	stop_streams(dice, AMDTP_OUT_STREAM, &rx_params);
+	snd_dice_transaction_clear_enable(dice);
+	release_resources(dice);
+	return err;
+}
+
 /*
  * MEMO: After this function, there're two states of streams:
  *  - None streams are running.
@@ -283,8 +340,6 @@ int snd_dice_stream_start_duplex(struct snd_dice *dice, unsigned int rate)
 {
 	unsigned int curr_rate;
 	unsigned int i;
-	struct reg_params tx_params, rx_params;
-	bool need_to_start = false;
 	unsigned int mode;
 	int err;
 
@@ -311,7 +366,7 @@ int snd_dice_stream_start_duplex(struct snd_dice *dice, unsigned int rate)
 			break;
 	}
 	if (i < MAX_STREAMS)
-		need_to_start = true;
+		goto restart;
 
 	/* Check required streams are running or not. */
 	err = snd_dice_stream_get_rate_mode(dice, rate, &mode);
@@ -326,61 +381,11 @@ int snd_dice_stream_start_duplex(struct snd_dice *dice, unsigned int rate)
 			break;
 	}
 	if (i < MAX_STREAMS)
-		need_to_start = true;
-
-	if (need_to_start) {
-		err = get_register_params(dice, &tx_params, &rx_params);
-		if (err < 0)
-			return err;
-
-		/* Stop transmission. */
-		snd_dice_transaction_clear_enable(dice);
-		stop_streams(dice, AMDTP_IN_STREAM, &tx_params);
-		stop_streams(dice, AMDTP_OUT_STREAM, &rx_params);
-		release_resources(dice);
+		goto restart;
 
-		err = ensure_phase_lock(dice);
-		if (err < 0) {
-			dev_err(&dice->unit->device,
-				"fail to ensure phase lock\n");
-			goto error;
-		}
-
-		/* Start both streams. */
-		err = start_streams(dice, AMDTP_IN_STREAM, rate, &tx_params);
-		if (err < 0)
-			goto error;
-		err = start_streams(dice, AMDTP_OUT_STREAM, rate, &rx_params);
-		if (err < 0)
-			goto error;
-
-		err = snd_dice_transaction_set_enable(dice);
-		if (err < 0) {
-			dev_err(&dice->unit->device,
-				"fail to enable interface\n");
-			goto error;
-		}
-
-		for (i = 0; i < MAX_STREAMS; i++) {
-			if ((i < tx_params.count &&
-			    !amdtp_stream_wait_callback(&dice->tx_stream[i],
-							CALLBACK_TIMEOUT)) ||
-			    (i < rx_params.count &&
-			     !amdtp_stream_wait_callback(&dice->rx_stream[i],
-							 CALLBACK_TIMEOUT))) {
-				err = -ETIMEDOUT;
-				goto error;
-			}
-		}
-	}
-
-	return err;
-error:
-	snd_dice_transaction_clear_enable(dice);
-	stop_streams(dice, AMDTP_IN_STREAM, &tx_params);
-	stop_streams(dice, AMDTP_OUT_STREAM, &rx_params);
-	release_resources(dice);
-	return err;
+	return 0;
+restart:
+	return start_duplex_streams(dice, rate);
 }
 
 /*
-- 
2.14.1

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

* [PATCH v2 10/13] ALSA: dice: enable to change current sampling transmission frequency
  2018-04-29  6:50 [PATCH v2 00/13] ALSA: dice: use cache of stream formats to generate PCM rules/constraints Takashi Sakamoto
                   ` (8 preceding siblings ...)
  2018-04-29  6:50 ` [PATCH v2 09/13] ALSA: dice: add a helper function to restart all of available streams Takashi Sakamoto
@ 2018-04-29  6:50 ` Takashi Sakamoto
  2018-04-29  6:50 ` [PATCH v2 11/13] ALSA: dice: use stream formats to add MIDI substreams Takashi Sakamoto
                   ` (2 subsequent siblings)
  12 siblings, 0 replies; 21+ messages in thread
From: Takashi Sakamoto @ 2018-04-29  6:50 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel, ffado-devel

This is a preparation for userspace applications to change current sampling
transmission frequency via ALSA PCM interface.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/dice/dice-stream.c | 47 +++++++++++++++++++++++++++++++++++----
 1 file changed, 43 insertions(+), 4 deletions(-)

diff --git a/sound/firewire/dice/dice-stream.c b/sound/firewire/dice/dice-stream.c
index 6a73528013d3..37c6c5d152dc 100644
--- a/sound/firewire/dice/dice-stream.c
+++ b/sound/firewire/dice/dice-stream.c
@@ -52,9 +52,11 @@ int snd_dice_stream_get_rate_mode(struct snd_dice *dice, unsigned int rate,
  * This operation has an effect to synchronize GLOBAL_STATUS/GLOBAL_SAMPLE_RATE
  * to GLOBAL_STATUS. Especially, just after powering on, these are different.
  */
-static int ensure_phase_lock(struct snd_dice *dice)
+static int ensure_phase_lock(struct snd_dice *dice, unsigned int rate)
 {
 	__be32 reg, nominal;
+	u32 data;
+	int i;
 	int err;
 
 	err = snd_dice_transaction_read_global(dice, GLOBAL_CLOCK_SELECT,
@@ -62,9 +64,21 @@ static int ensure_phase_lock(struct snd_dice *dice)
 	if (err < 0)
 		return err;
 
+	data = be32_to_cpu(reg);
+
+	data &= ~CLOCK_RATE_MASK;
+	for (i = 0; i < ARRAY_SIZE(snd_dice_rates); ++i) {
+		if (snd_dice_rates[i] == rate)
+			break;
+	}
+	if (i == ARRAY_SIZE(snd_dice_rates))
+		return -EINVAL;
+	data |= i << CLOCK_RATE_SHIFT;
+
 	if (completion_done(&dice->clock_accepted))
 		reinit_completion(&dice->clock_accepted);
 
+	reg = cpu_to_be32(data);
 	err = snd_dice_transaction_write_global(dice, GLOBAL_CLOCK_SELECT,
 						&reg, sizeof(reg));
 	if (err < 0)
@@ -210,6 +224,7 @@ static int start_streams(struct snd_dice *dice, enum amdtp_stream_direction dir,
 			 unsigned int rate, struct reg_params *params)
 {
 	__be32 reg[2];
+	unsigned int mode;
 	unsigned int i, pcm_chs, midi_ports;
 	struct amdtp_stream *streams;
 	struct fw_iso_resources *resources;
@@ -224,12 +239,23 @@ static int start_streams(struct snd_dice *dice, enum amdtp_stream_direction dir,
 		resources = dice->rx_resources;
 	}
 
+	err = snd_dice_stream_get_rate_mode(dice, rate, &mode);
+	if (err < 0)
+		return err;
+
 	for (i = 0; i < params->count; i++) {
+		unsigned int pcm_cache;
+		unsigned int midi_cache;
+
 		if (dir == AMDTP_IN_STREAM) {
+			pcm_cache = dice->tx_pcm_chs[i][mode];
+			midi_cache = dice->tx_midi_ports[i];
 			err = snd_dice_transaction_read_tx(dice,
 					params->size * i + TX_NUMBER_AUDIO,
 					reg, sizeof(reg));
 		} else {
+			pcm_cache = dice->rx_pcm_chs[i][mode];
+			midi_cache = dice->rx_midi_ports[i];
 			err = snd_dice_transaction_read_rx(dice,
 					params->size * i + RX_NUMBER_AUDIO,
 					reg, sizeof(reg));
@@ -239,6 +265,14 @@ static int start_streams(struct snd_dice *dice, enum amdtp_stream_direction dir,
 		pcm_chs = be32_to_cpu(reg[0]);
 		midi_ports = be32_to_cpu(reg[1]);
 
+		/* These are important for developer of this driver. */
+		if (pcm_chs != pcm_cache || midi_ports != midi_cache) {
+			dev_info(&dice->unit->device,
+				 "cache mismatch: pcm: %u:%u, midi: %u:%u\n",
+				 pcm_chs, pcm_cache, midi_ports, midi_cache);
+			return -EPROTO;
+		}
+
 		err = keep_resources(dice, dir, i, rate, pcm_chs, midi_ports);
 		if (err < 0)
 			return err;
@@ -290,12 +324,17 @@ static int start_duplex_streams(struct snd_dice *dice, unsigned int rate)
 	snd_dice_transaction_clear_enable(dice);
 	release_resources(dice);
 
-	err = ensure_phase_lock(dice);
+	err = ensure_phase_lock(dice, rate);
 	if (err < 0) {
 		dev_err(&dice->unit->device, "fail to ensure phase lock\n");
 		return err;
 	}
 
+	/* Likely to have changed stream formats. */
+	err = get_register_params(dice, &tx_params, &rx_params);
+	if (err < 0)
+		return err;
+
 	/* Start both streams. */
 	err = start_streams(dice, AMDTP_IN_STREAM, rate, &tx_params);
 	if (err < 0)
@@ -356,7 +395,7 @@ int snd_dice_stream_start_duplex(struct snd_dice *dice, unsigned int rate)
 	if (rate == 0)
 		rate = curr_rate;
 	if (rate != curr_rate)
-		return -EINVAL;
+		goto restart;
 
 	/* Check error of packet streaming. */
 	for (i = 0; i < MAX_STREAMS; ++i) {
@@ -550,7 +589,7 @@ int snd_dice_stream_detect_current_formats(struct snd_dice *dice)
 	 * invalid stream formats. Selecting clock parameters have an effect
 	 * for the unit to refine it.
 	 */
-	err = ensure_phase_lock(dice);
+	err = ensure_phase_lock(dice, rate);
 	if (err < 0)
 		return err;
 
-- 
2.14.1

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

* [PATCH v2 11/13] ALSA: dice: use stream formats to add MIDI substreams
  2018-04-29  6:50 [PATCH v2 00/13] ALSA: dice: use cache of stream formats to generate PCM rules/constraints Takashi Sakamoto
                   ` (9 preceding siblings ...)
  2018-04-29  6:50 ` [PATCH v2 10/13] ALSA: dice: enable to change current sampling transmission frequency Takashi Sakamoto
@ 2018-04-29  6:50 ` Takashi Sakamoto
  2018-04-29  6:50 ` [PATCH v2 12/13] ALSA: dice: use cache for PCM constraints and rules Takashi Sakamoto
  2018-04-29  6:50 ` [PATCH v2 13/13] ALSA: dice: remove local frag of force_two_pcms Takashi Sakamoto
  12 siblings, 0 replies; 21+ messages in thread
From: Takashi Sakamoto @ 2018-04-29  6:50 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel, ffado-devel

In former commits, proxy structure gets members for cache of stream
formats. The cache can be used to count the number of MIDI substreams
to add.

This commit uses the cache for this purpose.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/dice/dice-midi.c | 23 +++++++----------------
 1 file changed, 7 insertions(+), 16 deletions(-)

diff --git a/sound/firewire/dice/dice-midi.c b/sound/firewire/dice/dice-midi.c
index 8ff6da3c51f7..84eca8a51a02 100644
--- a/sound/firewire/dice/dice-midi.c
+++ b/sound/firewire/dice/dice-midi.c
@@ -101,27 +101,18 @@ int snd_dice_create_midi(struct snd_dice *dice)
 		.close		= midi_close,
 		.trigger	= midi_playback_trigger,
 	};
-	__be32 reg;
 	struct snd_rawmidi *rmidi;
 	struct snd_rawmidi_str *str;
 	unsigned int midi_in_ports, midi_out_ports;
+	int i;
 	int err;
 
-	/*
-	 * Use the number of MIDI conformant data channel at current sampling
-	 * transfer frequency.
-	 */
-	err = snd_dice_transaction_read_tx(dice, TX_NUMBER_MIDI,
-					   &reg, sizeof(reg));
-	if (err < 0)
-		return err;
-	midi_in_ports = be32_to_cpu(reg);
-
-	err = snd_dice_transaction_read_rx(dice, RX_NUMBER_MIDI,
-					   &reg, sizeof(reg));
-	if (err < 0)
-		return err;
-	midi_out_ports = be32_to_cpu(reg);
+	midi_in_ports = 0;
+	midi_out_ports = 0;
+	for (i = 0; i < MAX_STREAMS; ++i) {
+		midi_in_ports = max(midi_in_ports, dice->tx_midi_ports[i]);
+		midi_out_ports = max(midi_out_ports, dice->rx_midi_ports[i]);
+	}
 
 	if (midi_in_ports + midi_out_ports == 0)
 		return 0;
-- 
2.14.1

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

* [PATCH v2 12/13] ALSA: dice: use cache for PCM constraints and rules
  2018-04-29  6:50 [PATCH v2 00/13] ALSA: dice: use cache of stream formats to generate PCM rules/constraints Takashi Sakamoto
                   ` (10 preceding siblings ...)
  2018-04-29  6:50 ` [PATCH v2 11/13] ALSA: dice: use stream formats to add MIDI substreams Takashi Sakamoto
@ 2018-04-29  6:50 ` Takashi Sakamoto
  2018-04-29  6:50 ` [PATCH v2 13/13] ALSA: dice: remove local frag of force_two_pcms Takashi Sakamoto
  12 siblings, 0 replies; 21+ messages in thread
From: Takashi Sakamoto @ 2018-04-29  6:50 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel, ffado-devel

In former commits, proxy structure gets members for cache of stream
formats. The cache allows to apply correct constraints and rules to
runtime of PCM substream. They allows userspace applications to change
current sampling transmission frequency.

This commit uses the cacher for the PCM constraints and rules.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/dice/dice-pcm.c | 224 ++++++++++++++++++++++++++++++-----------
 1 file changed, 167 insertions(+), 57 deletions(-)

diff --git a/sound/firewire/dice/dice-pcm.c b/sound/firewire/dice/dice-pcm.c
index 7cb9e9713ac3..d0c323e16a39 100644
--- a/sound/firewire/dice/dice-pcm.c
+++ b/sound/firewire/dice/dice-pcm.c
@@ -9,43 +9,112 @@
 
 #include "dice.h"
 
+static int dice_rate_constraint(struct snd_pcm_hw_params *params,
+				struct snd_pcm_hw_rule *rule)
+{
+	struct snd_pcm_substream *substream = rule->private;
+	struct snd_dice *dice = substream->private_data;
+	unsigned int index = substream->pcm->device;
+
+	const struct snd_interval *c =
+		hw_param_interval_c(params, SNDRV_PCM_HW_PARAM_CHANNELS);
+	struct snd_interval *r =
+		hw_param_interval(params, SNDRV_PCM_HW_PARAM_RATE);
+	struct snd_interval rates = {
+		.min = UINT_MAX, .max = 0, .integer = 1
+	};
+	unsigned int *pcm_channels;
+	unsigned int i, rate, mode;
+
+	if (substream->stream == SNDRV_PCM_STREAM_CAPTURE)
+		pcm_channels = dice->tx_pcm_chs[index];
+	else
+		pcm_channels = dice->rx_pcm_chs[index];
+
+	for (i = 0; i < ARRAY_SIZE(snd_dice_rates); ++i) {
+		rate = snd_dice_rates[i];
+		if (snd_dice_stream_get_rate_mode(dice, rate, &mode) < 0)
+			continue;
+
+		if (!snd_interval_test(c, pcm_channels[mode]))
+			continue;
+
+		rates.min = min(rates.min, rate);
+		rates.max = max(rates.max, rate);
+	}
+
+	return snd_interval_refine(r, &rates);
+}
+
+static int dice_channels_constraint(struct snd_pcm_hw_params *params,
+				    struct snd_pcm_hw_rule *rule)
+{
+	struct snd_pcm_substream *substream = rule->private;
+	struct snd_dice *dice = substream->private_data;
+	unsigned int index = substream->pcm->device;
+
+	const struct snd_interval *r =
+		hw_param_interval_c(params, SNDRV_PCM_HW_PARAM_RATE);
+	struct snd_interval *c =
+		hw_param_interval(params, SNDRV_PCM_HW_PARAM_CHANNELS);
+	struct snd_interval channels = {
+		.min = UINT_MAX, .max = 0, .integer = 1
+	};
+	unsigned int *pcm_channels;
+	unsigned int i, rate, mode;
+
+	if (substream->stream == SNDRV_PCM_STREAM_CAPTURE)
+		pcm_channels = dice->tx_pcm_chs[index];
+	else
+		pcm_channels = dice->rx_pcm_chs[index];
+
+	for (i = 0; i < ARRAY_SIZE(snd_dice_rates); ++i) {
+		rate = snd_dice_rates[i];
+		if (snd_dice_stream_get_rate_mode(dice, rate, &mode) < 0)
+			continue;
+
+		if (!snd_interval_test(r, rate))
+			continue;
+
+		channels.min = min(channels.min, pcm_channels[mode]);
+		channels.max = max(channels.max, pcm_channels[mode]);
+	}
+
+	return snd_interval_refine(c, &channels);
+}
+
 static int limit_channels_and_rates(struct snd_dice *dice,
 				    struct snd_pcm_runtime *runtime,
 				    enum amdtp_stream_direction dir,
-				    unsigned int index, unsigned int size)
+				    unsigned int index)
 {
 	struct snd_pcm_hardware *hw = &runtime->hw;
-	struct amdtp_stream *stream;
-	unsigned int rate;
-	__be32 reg;
-	int err;
-
-	/*
-	 * Retrieve current Multi Bit Linear Audio data channel and limit to
-	 * it.
-	 */
-	if (dir == AMDTP_IN_STREAM) {
-		stream = &dice->tx_stream[index];
-		err = snd_dice_transaction_read_tx(dice,
-				size * index + TX_NUMBER_AUDIO,
-				&reg, sizeof(reg));
-	} else {
-		stream = &dice->rx_stream[index];
-		err = snd_dice_transaction_read_rx(dice,
-				size * index + RX_NUMBER_AUDIO,
-				&reg, sizeof(reg));
+	unsigned int *pcm_channels;
+	unsigned int i;
+
+	if (dir == AMDTP_IN_STREAM)
+		pcm_channels = dice->tx_pcm_chs[index];
+	else
+		pcm_channels = dice->rx_pcm_chs[index];
+
+	hw->channels_min = UINT_MAX;
+	hw->channels_max = 0;
+
+	for (i = 0; i < ARRAY_SIZE(snd_dice_rates); ++i) {
+		unsigned int rate, mode, channels;
+
+		rate = snd_dice_rates[i];
+		if (snd_dice_stream_get_rate_mode(dice, rate, &mode) < 0)
+			continue;
+		hw->rates |= snd_pcm_rate_to_rate_bit(rate);
+
+		channels = pcm_channels[mode];
+		if (channels == 0)
+			continue;
+		hw->channels_min = min(hw->channels_min, channels);
+		hw->channels_max = max(hw->channels_max, channels);
 	}
-	if (err < 0)
-		return err;
-
-	hw->channels_min = hw->channels_max = be32_to_cpu(reg);
-
-	/* Retrieve current sampling transfer frequency and limit to it. */
-	err = snd_dice_transaction_get_rate(dice, &rate);
-	if (err < 0)
-		return err;
 
-	hw->rates = snd_pcm_rate_to_rate_bit(rate);
 	snd_pcm_limit_hw_rates(runtime);
 
 	return 0;
@@ -56,36 +125,34 @@ static int init_hw_info(struct snd_dice *dice,
 {
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	struct snd_pcm_hardware *hw = &runtime->hw;
+	unsigned int index = substream->pcm->device;
 	enum amdtp_stream_direction dir;
 	struct amdtp_stream *stream;
-	__be32 reg[2];
-	unsigned int count, size;
 	int err;
 
 	if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) {
 		hw->formats = AM824_IN_PCM_FORMAT_BITS;
 		dir = AMDTP_IN_STREAM;
-		stream = &dice->tx_stream[substream->pcm->device];
-		err = snd_dice_transaction_read_tx(dice, TX_NUMBER, reg,
-						   sizeof(reg));
+		stream = &dice->tx_stream[index];
 	} else {
 		hw->formats = AM824_OUT_PCM_FORMAT_BITS;
 		dir = AMDTP_OUT_STREAM;
-		stream = &dice->rx_stream[substream->pcm->device];
-		err = snd_dice_transaction_read_rx(dice, RX_NUMBER, reg,
-						   sizeof(reg));
+		stream = &dice->rx_stream[index];
 	}
 
+	err = limit_channels_and_rates(dice, substream->runtime, dir,
+				       index);
 	if (err < 0)
 		return err;
 
-	count = min_t(unsigned int, be32_to_cpu(reg[0]), MAX_STREAMS);
-	if (substream->pcm->device >= count)
-		return -ENXIO;
-
-	size = be32_to_cpu(reg[1]) * 4;
-	err = limit_channels_and_rates(dice, substream->runtime, dir,
-				       substream->pcm->device, size);
+	err = snd_pcm_hw_rule_add(runtime, 0, SNDRV_PCM_HW_PARAM_RATE,
+				  dice_rate_constraint, substream,
+				  SNDRV_PCM_HW_PARAM_CHANNELS, -1);
+	if (err < 0)
+		return err;
+	err = snd_pcm_hw_rule_add(runtime, 0, SNDRV_PCM_HW_PARAM_CHANNELS,
+				  dice_channels_constraint, substream,
+				  SNDRV_PCM_HW_PARAM_RATE, -1);
 	if (err < 0)
 		return err;
 
@@ -95,6 +162,8 @@ static int init_hw_info(struct snd_dice *dice,
 static int pcm_open(struct snd_pcm_substream *substream)
 {
 	struct snd_dice *dice = substream->private_data;
+	unsigned int source;
+	bool internal;
 	int err;
 
 	err = snd_dice_stream_lock_try(dice);
@@ -105,6 +174,43 @@ static int pcm_open(struct snd_pcm_substream *substream)
 	if (err < 0)
 		goto err_locked;
 
+	err = snd_dice_transaction_get_clock_source(dice, &source);
+	if (err < 0)
+		goto err_locked;
+	switch (source) {
+	case CLOCK_SOURCE_AES1:
+	case CLOCK_SOURCE_AES2:
+	case CLOCK_SOURCE_AES3:
+	case CLOCK_SOURCE_AES4:
+	case CLOCK_SOURCE_AES_ANY:
+	case CLOCK_SOURCE_ADAT:
+	case CLOCK_SOURCE_TDIF:
+	case CLOCK_SOURCE_WC:
+		internal = false;
+		break;
+	default:
+		internal = true;
+		break;
+	}
+
+	/*
+	 * When source of clock is not internal or any PCM streams are running,
+	 * available sampling rate is limited at current sampling rate.
+	 */
+	if (!internal ||
+	    amdtp_stream_pcm_running(&dice->tx_stream[0]) ||
+	    amdtp_stream_pcm_running(&dice->tx_stream[1]) ||
+	    amdtp_stream_pcm_running(&dice->rx_stream[0]) ||
+	    amdtp_stream_pcm_running(&dice->rx_stream[1])) {
+		unsigned int rate;
+
+		err = snd_dice_transaction_get_rate(dice, &rate);
+		if (err < 0)
+			goto err_locked;
+		substream->runtime->hw.rate_min = rate;
+		substream->runtime->hw.rate_max = rate;
+	}
+
 	snd_pcm_set_sync(substream);
 end:
 	return err;
@@ -318,7 +424,6 @@ int snd_dice_create_pcm(struct snd_dice *dice)
 		.page      = snd_pcm_lib_get_vmalloc_page,
 		.mmap      = snd_pcm_lib_mmap_vmalloc,
 	};
-	__be32 reg;
 	struct snd_pcm *pcm;
 	unsigned int i, max_capture, max_playback, capture, playback;
 	int err;
@@ -327,18 +432,23 @@ int snd_dice_create_pcm(struct snd_dice *dice)
 	if (dice->force_two_pcms) {
 		max_capture = max_playback = 2;
 	} else {
+		int j;
 		max_capture = max_playback = 0;
-		err = snd_dice_transaction_read_tx(dice, TX_NUMBER, &reg,
-						   sizeof(reg));
-		if (err < 0)
-			return err;
-		max_capture = min_t(unsigned int, be32_to_cpu(reg), MAX_STREAMS);
-
-		err = snd_dice_transaction_read_rx(dice, RX_NUMBER, &reg,
-						   sizeof(reg));
-		if (err < 0)
-			return err;
-		max_playback = min_t(unsigned int, be32_to_cpu(reg), MAX_STREAMS);
+		for (i = 0; i < MAX_STREAMS; ++i) {
+			for (j = 0; j < 3; ++j) {
+				if (dice->tx_pcm_chs[i][j] > 0) {
+					++max_capture;
+					break;
+				}
+			}
+
+			for (j = 0; j < 3; ++j) {
+				if (dice->rx_pcm_chs[i][j] > 0) {
+					++max_playback;
+					break;
+				}
+			}
+		}
 	}
 
 	for (i = 0; i < MAX_STREAMS; i++) {
-- 
2.14.1

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

* [PATCH v2 13/13] ALSA: dice: remove local frag of force_two_pcms
  2018-04-29  6:50 [PATCH v2 00/13] ALSA: dice: use cache of stream formats to generate PCM rules/constraints Takashi Sakamoto
                   ` (11 preceding siblings ...)
  2018-04-29  6:50 ` [PATCH v2 12/13] ALSA: dice: use cache for PCM constraints and rules Takashi Sakamoto
@ 2018-04-29  6:50 ` Takashi Sakamoto
  12 siblings, 0 replies; 21+ messages in thread
From: Takashi Sakamoto @ 2018-04-29  6:50 UTC (permalink / raw)
  To: clemens, tiwai; +Cc: alsa-devel, ffado-devel

At present, to add PCM substreams for each of available tx/rx streams,
this driver uses a condition based on model-name. This is not enough
to support unknown models.

In former commits, this driver gains cache of stream formats. For models
which support protocol extension, all of available steam formats are
cached. For known models, hard-coded stream formats are used to generate
the cache. For unknown models, stream formats at current mode of sampling
transmission frequency is cached.

Anyway, at least, the cached formats are used to expose constrains of PCM
substreams for userspace applications. Thus, The cached data can be also
used to add PCM substreams themselves, instead of the name-based
conditions.

This commit obsoletes local frag of force_two_pcms.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/dice/dice-pcm.c | 38 ++++++++------------------------------
 sound/firewire/dice/dice.c     | 38 --------------------------------------
 sound/firewire/dice/dice.h     |  2 --
 3 files changed, 8 insertions(+), 70 deletions(-)

diff --git a/sound/firewire/dice/dice-pcm.c b/sound/firewire/dice/dice-pcm.c
index d0c323e16a39..8e06f7234e82 100644
--- a/sound/firewire/dice/dice-pcm.c
+++ b/sound/firewire/dice/dice-pcm.c
@@ -425,40 +425,18 @@ int snd_dice_create_pcm(struct snd_dice *dice)
 		.mmap      = snd_pcm_lib_mmap_vmalloc,
 	};
 	struct snd_pcm *pcm;
-	unsigned int i, max_capture, max_playback, capture, playback;
+	unsigned int capture, playback;
+	int i, j;
 	int err;
 
-	/* Check whether PCM substreams are required. */
-	if (dice->force_two_pcms) {
-		max_capture = max_playback = 2;
-	} else {
-		int j;
-		max_capture = max_playback = 0;
-		for (i = 0; i < MAX_STREAMS; ++i) {
-			for (j = 0; j < 3; ++j) {
-				if (dice->tx_pcm_chs[i][j] > 0) {
-					++max_capture;
-					break;
-				}
-			}
-
-			for (j = 0; j < 3; ++j) {
-				if (dice->rx_pcm_chs[i][j] > 0) {
-					++max_playback;
-					break;
-				}
-			}
-		}
-	}
-
 	for (i = 0; i < MAX_STREAMS; i++) {
 		capture = playback = 0;
-		if (i < max_capture)
-			capture = 1;
-		if (i < max_playback)
-			playback = 1;
-		if (capture == 0 && playback == 0)
-			break;
+		for (j = 0; j < 3; ++j) {
+			if (dice->tx_pcm_chs[i][j] > 0)
+				capture = 1;
+			if (dice->rx_pcm_chs[i][j] > 0)
+				playback = 1;
+		}
 
 		err = snd_pcm_new(dice->card, "DICE", i, playback, capture,
 				  &pcm);
diff --git a/sound/firewire/dice/dice.c b/sound/firewire/dice/dice.c
index 6d55a62ec89e..40f7a32e4893 100644
--- a/sound/firewire/dice/dice.c
+++ b/sound/firewire/dice/dice.c
@@ -24,36 +24,6 @@ MODULE_LICENSE("GPL v2");
 
 #define MODEL_ALESIS_IO_BOTH	0x000001
 
-/*
- * Some models support several isochronous channels, while these streams are not
- * always available. In this case, add the model name to this list.
- */
-static bool force_two_pcm_support(struct fw_unit *unit)
-{
-	static const char *const models[] = {
-		/* TC Electronic models. */
-		"StudioKonnekt48",
-		/* Focusrite models. */
-		"SAFFIRE_PRO_40",
-		"LIQUID_SAFFIRE_56",
-		"SAFFIRE_PRO_40_1",
-	};
-	char model[32];
-	unsigned int i;
-	int err;
-
-	err = fw_csr_string(unit->directory, CSR_MODEL, model, sizeof(model));
-	if (err < 0)
-		return false;
-
-	for (i = 0; i < ARRAY_SIZE(models); i++) {
-		if (strcmp(models[i], model) == 0)
-			break;
-	}
-
-	return i < ARRAY_SIZE(models);
-}
-
 static int check_dice_category(struct fw_unit *unit)
 {
 	struct fw_device *device = fw_parent_device(unit);
@@ -79,11 +49,6 @@ static int check_dice_category(struct fw_unit *unit)
 		}
 	}
 
-	if (vendor == OUI_FOCUSRITE || vendor == OUI_TCELECTRONIC) {
-		if (force_two_pcm_support(unit))
-			return 0;
-	}
-
 	if (vendor == OUI_WEISS)
 		category = WEISS_CATEGORY_ID;
 	else if (vendor == OUI_LOUD)
@@ -190,9 +155,6 @@ static void do_registration(struct work_struct *work)
 	if (err < 0)
 		return;
 
-	if (force_two_pcm_support(dice->unit))
-		dice->force_two_pcms = true;
-
 	err = snd_dice_transaction_init(dice);
 	if (err < 0)
 		goto error;
diff --git a/sound/firewire/dice/dice.h b/sound/firewire/dice/dice.h
index 7b1e2396eadc..0f5927e89bc9 100644
--- a/sound/firewire/dice/dice.h
+++ b/sound/firewire/dice/dice.h
@@ -106,8 +106,6 @@ struct snd_dice {
 	bool global_enabled;
 	struct completion clock_accepted;
 	unsigned int substreams_counter;
-
-	bool force_two_pcms;
 };
 
 enum snd_dice_addr_type {
-- 
2.14.1

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

* Re: [PATCH v2 04/13] ALSA: dice: cache stream formats at current mode of sampling transmission frequency
  2018-04-29  6:50 ` [PATCH v2 04/13] ALSA: dice: cache stream formats at current mode of sampling transmission frequency Takashi Sakamoto
@ 2018-04-30 15:10   ` Takashi Iwai
  2018-05-01  3:02     ` Takashi Sakamoto
  0 siblings, 1 reply; 21+ messages in thread
From: Takashi Iwai @ 2018-04-30 15:10 UTC (permalink / raw)
  To: Takashi Sakamoto; +Cc: ffado-devel, alsa-devel, clemens

On Sun, 29 Apr 2018 08:50:23 +0200,
Takashi Sakamoto wrote:
> 
> In former commits, proxy structure get members for cache of stream
> formats. This commit fills the cache with stream formats at current mode
> of sampling transmission frequency.
> 
> Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
> ---
>  sound/firewire/dice/dice-stream.c | 76 +++++++++++++++++++++++++++++++++++++++
>  sound/firewire/dice/dice.c        |  4 +++
>  sound/firewire/dice/dice.h        |  3 ++
>  3 files changed, 83 insertions(+)
> 
> diff --git a/sound/firewire/dice/dice-stream.c b/sound/firewire/dice/dice-stream.c
> index 928a255bfc35..2a9f0cd994a5 100644
> --- a/sound/firewire/dice/dice-stream.c
> +++ b/sound/firewire/dice/dice-stream.c
> @@ -30,6 +30,24 @@ const unsigned int snd_dice_rates[SND_DICE_RATES_COUNT] = {
>  	[6] = 192000,
>  };
>  
> +int snd_dice_stream_get_rate_mode(struct snd_dice *dice, unsigned int rate,
> +				  unsigned int *mode)
> +{
> +	int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(snd_dice_rates); i++) {
> +		if (!(dice->clock_caps & BIT(i)))
> +			continue;
> +		if (snd_dice_rates[i] != rate)
> +			continue;
> +
> +		*mode = (i - 1) / 2;

What if i=0?  It'll be a negative value?


thanks,

Takashi

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

* Re: [PATCH v2 03/13] ALSA: dice: add proc node for stream formation
  2018-04-29  6:50 ` [PATCH v2 03/13] ALSA: dice: add proc node for stream formation Takashi Sakamoto
@ 2018-04-30 15:12   ` Takashi Iwai
  2018-05-01  3:06     ` Takashi Sakamoto
  0 siblings, 1 reply; 21+ messages in thread
From: Takashi Iwai @ 2018-04-30 15:12 UTC (permalink / raw)
  To: Takashi Sakamoto; +Cc: ffado-devel, alsa-devel, clemens

On Sun, 29 Apr 2018 08:50:22 +0200,
Takashi Sakamoto wrote:
> 
> Products with DICE interface in market can support variable stream
> formats for three levels of sampling transmission frequencies. To
> record these formats, a proxy structure got several fields in former
> commit.
> 
> This commit adds a proc node to output the stream formats for debugging
> purpose.
> 
> Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
> ---
>  sound/firewire/dice/dice-proc.c | 35 +++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/sound/firewire/dice/dice-proc.c b/sound/firewire/dice/dice-proc.c
> index 43b130b7aa07..f967e0ec7598 100644
> --- a/sound/firewire/dice/dice-proc.c
> +++ b/sound/firewire/dice/dice-proc.c
> @@ -243,6 +243,40 @@ static void dice_proc_read(struct snd_info_entry *entry,
>  	}
>  }
>  
> +static void dice_proc_read_formation(struct snd_info_entry *entry,
> +				     struct snd_info_buffer *buffer)
> +{
> +	static const char *const rate_labels[] = {
> +		[0] = "low",
> +		[1] = "middle",
> +		[2] = "high",
> +	};

This doesn't look smart :)

How about defining these three numbers as constants (or macros)?
Also the magic number 3 appearing everywhere can be defined properly,
too.


Takashi

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

* Re: [PATCH v2 04/13] ALSA: dice: cache stream formats at current mode of sampling transmission frequency
  2018-04-30 15:10   ` Takashi Iwai
@ 2018-05-01  3:02     ` Takashi Sakamoto
  2018-05-01  6:54       ` Takashi Iwai
  0 siblings, 1 reply; 21+ messages in thread
From: Takashi Sakamoto @ 2018-05-01  3:02 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: ffado-devel, alsa-devel, clemens

Hi,

On May 1 2018 00:10, Takashi Iwai wrote:
> On Sun, 29 Apr 2018 08:50:23 +0200,
> Takashi Sakamoto wrote:
>>
>> In former commits, proxy structure get members for cache of stream
>> formats. This commit fills the cache with stream formats at current mode
>> of sampling transmission frequency.
>>
>> Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
>> ---
>>   sound/firewire/dice/dice-stream.c | 76 +++++++++++++++++++++++++++++++++++++++
>>   sound/firewire/dice/dice.c        |  4 +++
>>   sound/firewire/dice/dice.h        |  3 ++
>>   3 files changed, 83 insertions(+)
>>
>> diff --git a/sound/firewire/dice/dice-stream.c b/sound/firewire/dice/dice-stream.c
>> index 928a255bfc35..2a9f0cd994a5 100644
>> --- a/sound/firewire/dice/dice-stream.c
>> +++ b/sound/firewire/dice/dice-stream.c
>> @@ -30,6 +30,24 @@ const unsigned int snd_dice_rates[SND_DICE_RATES_COUNT] = {
>>   	[6] = 192000,
>>   };
>>   
>> +int snd_dice_stream_get_rate_mode(struct snd_dice *dice, unsigned int rate,
>> +				  unsigned int *mode)
>> +{
>> +	int i;
>> +
>> +	for (i = 0; i < ARRAY_SIZE(snd_dice_rates); i++) {
>> +		if (!(dice->clock_caps & BIT(i)))
>> +			continue;
>> +		if (snd_dice_rates[i] != rate)
>> +			continue;
>> +
>> +		*mode = (i - 1) / 2;
> 
> What if i=0?  It'll be a negative value?

Yes. But division by 2 to -1 results in 0 because in C language 
specification 'truncate toward zero'[1] is applied to discard
fractional part. Then, the result is assigned to value of 'unsigned int'
type. As a result:

0: 32000/44100/48000
1: 88200/96000
2: 176400/192000

This is what I expect.

[1] footnote for '6.5.5 Multiplicative operators' clause in ISO/IEC 9899.


Thanks

Takashi Sakamoto

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

* Re: [PATCH v2 03/13] ALSA: dice: add proc node for stream formation
  2018-04-30 15:12   ` Takashi Iwai
@ 2018-05-01  3:06     ` Takashi Sakamoto
  0 siblings, 0 replies; 21+ messages in thread
From: Takashi Sakamoto @ 2018-05-01  3:06 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: ffado-devel, alsa-devel, clemens

Hi,

On May 1 2018 00:12, Takashi Iwai wrote:
> On Sun, 29 Apr 2018 08:50:22 +0200,
> Takashi Sakamoto wrote:
>>
>> Products with DICE interface in market can support variable stream
>> formats for three levels of sampling transmission frequencies. To
>> record these formats, a proxy structure got several fields in former
>> commit.
>>
>> This commit adds a proc node to output the stream formats for debugging
>> purpose.
>>
>> Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
>> ---
>>   sound/firewire/dice/dice-proc.c | 35 +++++++++++++++++++++++++++++++++++
>>   1 file changed, 35 insertions(+)
>>
>> diff --git a/sound/firewire/dice/dice-proc.c b/sound/firewire/dice/dice-proc.c
>> index 43b130b7aa07..f967e0ec7598 100644
>> --- a/sound/firewire/dice/dice-proc.c
>> +++ b/sound/firewire/dice/dice-proc.c
>> @@ -243,6 +243,40 @@ static void dice_proc_read(struct snd_info_entry *entry,
>>   	}
>>   }
>>   
>> +static void dice_proc_read_formation(struct snd_info_entry *entry,
>> +				     struct snd_info_buffer *buffer)
>> +{
>> +	static const char *const rate_labels[] = {
>> +		[0] = "low",
>> +		[1] = "middle",
>> +		[2] = "high",
>> +	};
> 
> This doesn't look smart :)
> 
> How about defining these three numbers as constants (or macros)?
> Also the magic number 3 appearing everywhere can be defined properly,
> too.

Indeed. I was a bit rude. I'm OK to utilize constants for this purpose.


Thanks

Takashi Sakamoto

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

* Re: [PATCH v2 04/13] ALSA: dice: cache stream formats at current mode of sampling transmission frequency
  2018-05-01  3:02     ` Takashi Sakamoto
@ 2018-05-01  6:54       ` Takashi Iwai
  2018-05-01  9:14         ` Takashi Sakamoto
  0 siblings, 1 reply; 21+ messages in thread
From: Takashi Iwai @ 2018-05-01  6:54 UTC (permalink / raw)
  To: Takashi Sakamoto; +Cc: ffado-devel, alsa-devel, clemens

On Tue, 01 May 2018 05:02:09 +0200,
Takashi Sakamoto wrote:
> 
> Hi,
> 
> On May 1 2018 00:10, Takashi Iwai wrote:
> > On Sun, 29 Apr 2018 08:50:23 +0200,
> > Takashi Sakamoto wrote:
> >>
> >> In former commits, proxy structure get members for cache of stream
> >> formats. This commit fills the cache with stream formats at current mode
> >> of sampling transmission frequency.
> >>
> >> Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
> >> ---
> >>   sound/firewire/dice/dice-stream.c | 76 +++++++++++++++++++++++++++++++++++++++
> >>   sound/firewire/dice/dice.c        |  4 +++
> >>   sound/firewire/dice/dice.h        |  3 ++
> >>   3 files changed, 83 insertions(+)
> >>
> >> diff --git a/sound/firewire/dice/dice-stream.c b/sound/firewire/dice/dice-stream.c
> >> index 928a255bfc35..2a9f0cd994a5 100644
> >> --- a/sound/firewire/dice/dice-stream.c
> >> +++ b/sound/firewire/dice/dice-stream.c
> >> @@ -30,6 +30,24 @@ const unsigned int snd_dice_rates[SND_DICE_RATES_COUNT] = {
> >>   	[6] = 192000,
> >>   };
> >>   +int snd_dice_stream_get_rate_mode(struct snd_dice *dice,
> >> unsigned int rate,
> >> +				  unsigned int *mode)
> >> +{
> >> +	int i;
> >> +
> >> +	for (i = 0; i < ARRAY_SIZE(snd_dice_rates); i++) {
> >> +		if (!(dice->clock_caps & BIT(i)))
> >> +			continue;
> >> +		if (snd_dice_rates[i] != rate)
> >> +			continue;
> >> +
> >> +		*mode = (i - 1) / 2;
> >
> > What if i=0?  It'll be a negative value?
> 
> Yes. But division by 2 to -1 results in 0 because in C language
> specification 'truncate toward zero'[1] is applied to discard
> fractional part. Then, the result is assigned to value of 'unsigned int'
> type. As a result:
> 
> 0: 32000/44100/48000
> 1: 88200/96000
> 2: 176400/192000
> 
> This is what I expect.
> 
> [1] footnote for '6.5.5 Multiplicative operators' clause in ISO/IEC 9899.

This is too tricky.  The result would change dramatically when i were
declared as unsigned.

And I can think of someone trying to change it unsigned because of the
comparison against ARRAY_SIZE() (we've got gcc warnings for that in
the past).

Please make either it more robust or put a proper comment.


thanks,

Takashi

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

* Re: [PATCH v2 04/13] ALSA: dice: cache stream formats at current mode of sampling transmission frequency
  2018-05-01  6:54       ` Takashi Iwai
@ 2018-05-01  9:14         ` Takashi Sakamoto
  2018-05-01  9:25           ` Takashi Iwai
  0 siblings, 1 reply; 21+ messages in thread
From: Takashi Sakamoto @ 2018-05-01  9:14 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: ffado-devel, alsa-devel, clemens

Hi,

On May 1 2018 15:54, Takashi Iwai wrote:
> On Tue, 01 May 2018 05:02:09 +0200,
> Takashi Sakamoto wrote:
>>
>> Hi,
>>
>> On May 1 2018 00:10, Takashi Iwai wrote:
>>> On Sun, 29 Apr 2018 08:50:23 +0200,
>>> Takashi Sakamoto wrote:
>>>>
>>>> In former commits, proxy structure get members for cache of stream
>>>> formats. This commit fills the cache with stream formats at current mode
>>>> of sampling transmission frequency.
>>>>
>>>> Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
>>>> ---
>>>>    sound/firewire/dice/dice-stream.c | 76 +++++++++++++++++++++++++++++++++++++++
>>>>    sound/firewire/dice/dice.c        |  4 +++
>>>>    sound/firewire/dice/dice.h        |  3 ++
>>>>    3 files changed, 83 insertions(+)
>>>>
>>>> diff --git a/sound/firewire/dice/dice-stream.c b/sound/firewire/dice/dice-stream.c
>>>> index 928a255bfc35..2a9f0cd994a5 100644
>>>> --- a/sound/firewire/dice/dice-stream.c
>>>> +++ b/sound/firewire/dice/dice-stream.c
>>>> @@ -30,6 +30,24 @@ const unsigned int snd_dice_rates[SND_DICE_RATES_COUNT] = {
>>>>    	[6] = 192000,
>>>>    };
>>>>    +int snd_dice_stream_get_rate_mode(struct snd_dice *dice,
>>>> unsigned int rate,
>>>> +				  unsigned int *mode)
>>>> +{
>>>> +	int i;
>>>> +
>>>> +	for (i = 0; i < ARRAY_SIZE(snd_dice_rates); i++) {
>>>> +		if (!(dice->clock_caps & BIT(i)))
>>>> +			continue;
>>>> +		if (snd_dice_rates[i] != rate)
>>>> +			continue;
>>>> +
>>>> +		*mode = (i - 1) / 2;
>>>
>>> What if i=0?  It'll be a negative value?
>>
>> Yes. But division by 2 to -1 results in 0 because in C language
>> specification 'truncate toward zero'[1] is applied to discard
>> fractional part. Then, the result is assigned to value of 'unsigned int'
>> type. As a result:
>>
>> 0: 32000/44100/48000
>> 1: 88200/96000
>> 2: 176400/192000
>>
>> This is what I expect.
>>
>> [1] footnote for '6.5.5 Multiplicative operators' clause in ISO/IEC 9899.
> 
> This is too tricky.  The result would change dramatically when i were
> declared as unsigned.
> 
> And I can think of someone trying to change it unsigned because of the
> comparison against ARRAY_SIZE() (we've got gcc warnings for that in
> the past).
> 
> Please make either it more robust or put a proper comment.

Hm. Any comment includes ambiguity, so I prefer the robust
representation with more consumption of .rodata section.

```
int snd_dice_stream_get_rate_mode(struct snd_dice *dice, unsigned int rate,
                                   enum snd_dice_rate_mode *mode)
{
	/* Corresponding to each entry in snd_dice_rates. */
         static const enum snd_dice_rate_mode modes[] = {
                 [0]     = SND_DICE_RATE_MODE_LOW,
                 [1]     = SND_DICE_RATE_MODE_LOW,
                 [2]     = SND_DICE_RATE_MODE_LOW,
                 [3]     = SND_DICE_RATE_MODE_MIDDLE,
                 [4]     = SND_DICE_RATE_MODE_MIDDLE,
                 [5]     = SND_DICE_RATE_MODE_HIGH,
                 [6]     = SND_DICE_RATE_MODE_HIGH,
         };
         int i;

         for (i = 0; i < ARRAY_SIZE(snd_dice_rates); i++) {
                 if (!(dice->clock_caps & BIT(i)))
                         continue;
                 if (snd_dice_rates[i] != rate)
                         continue;

                 *mode = modes[i];
                 return 0;
         }

         return -EINVAL;
}
```


Thanks

Takashi Sakamoto

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

* Re: [PATCH v2 04/13] ALSA: dice: cache stream formats at current mode of sampling transmission frequency
  2018-05-01  9:14         ` Takashi Sakamoto
@ 2018-05-01  9:25           ` Takashi Iwai
  0 siblings, 0 replies; 21+ messages in thread
From: Takashi Iwai @ 2018-05-01  9:25 UTC (permalink / raw)
  To: Takashi Sakamoto; +Cc: ffado-devel, alsa-devel, clemens

On Tue, 01 May 2018 11:14:56 +0200,
Takashi Sakamoto wrote:
> 
> Hi,
> 
> On May 1 2018 15:54, Takashi Iwai wrote:
> > On Tue, 01 May 2018 05:02:09 +0200,
> > Takashi Sakamoto wrote:
> >>
> >> Hi,
> >>
> >> On May 1 2018 00:10, Takashi Iwai wrote:
> >>> On Sun, 29 Apr 2018 08:50:23 +0200,
> >>> Takashi Sakamoto wrote:
> >>>>
> >>>> In former commits, proxy structure get members for cache of stream
> >>>> formats. This commit fills the cache with stream formats at current mode
> >>>> of sampling transmission frequency.
> >>>>
> >>>> Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
> >>>> ---
> >>>>    sound/firewire/dice/dice-stream.c | 76 +++++++++++++++++++++++++++++++++++++++
> >>>>    sound/firewire/dice/dice.c        |  4 +++
> >>>>    sound/firewire/dice/dice.h        |  3 ++
> >>>>    3 files changed, 83 insertions(+)
> >>>>
> >>>> diff --git a/sound/firewire/dice/dice-stream.c b/sound/firewire/dice/dice-stream.c
> >>>> index 928a255bfc35..2a9f0cd994a5 100644
> >>>> --- a/sound/firewire/dice/dice-stream.c
> >>>> +++ b/sound/firewire/dice/dice-stream.c
> >>>> @@ -30,6 +30,24 @@ const unsigned int snd_dice_rates[SND_DICE_RATES_COUNT] = {
> >>>>    	[6] = 192000,
> >>>>    };
> >>>>    +int snd_dice_stream_get_rate_mode(struct snd_dice *dice,
> >>>> unsigned int rate,
> >>>> +				  unsigned int *mode)
> >>>> +{
> >>>> +	int i;
> >>>> +
> >>>> +	for (i = 0; i < ARRAY_SIZE(snd_dice_rates); i++) {
> >>>> +		if (!(dice->clock_caps & BIT(i)))
> >>>> +			continue;
> >>>> +		if (snd_dice_rates[i] != rate)
> >>>> +			continue;
> >>>> +
> >>>> +		*mode = (i - 1) / 2;
> >>>
> >>> What if i=0?  It'll be a negative value?
> >>
> >> Yes. But division by 2 to -1 results in 0 because in C language
> >> specification 'truncate toward zero'[1] is applied to discard
> >> fractional part. Then, the result is assigned to value of 'unsigned int'
> >> type. As a result:
> >>
> >> 0: 32000/44100/48000
> >> 1: 88200/96000
> >> 2: 176400/192000
> >>
> >> This is what I expect.
> >>
> >> [1] footnote for '6.5.5 Multiplicative operators' clause in ISO/IEC 9899.
> >
> > This is too tricky.  The result would change dramatically when i were
> > declared as unsigned.
> >
> > And I can think of someone trying to change it unsigned because of the
> > comparison against ARRAY_SIZE() (we've got gcc warnings for that in
> > the past).
> >
> > Please make either it more robust or put a proper comment.
> 
> Hm. Any comment includes ambiguity, so I prefer the robust
> representation with more consumption of .rodata section.
> 
> ```
> int snd_dice_stream_get_rate_mode(struct snd_dice *dice, unsigned int rate,
>                                   enum snd_dice_rate_mode *mode)
> {
> 	/* Corresponding to each entry in snd_dice_rates. */
>         static const enum snd_dice_rate_mode modes[] = {
>                 [0]     = SND_DICE_RATE_MODE_LOW,
>                 [1]     = SND_DICE_RATE_MODE_LOW,
>                 [2]     = SND_DICE_RATE_MODE_LOW,
>                 [3]     = SND_DICE_RATE_MODE_MIDDLE,
>                 [4]     = SND_DICE_RATE_MODE_MIDDLE,
>                 [5]     = SND_DICE_RATE_MODE_HIGH,
>                 [6]     = SND_DICE_RATE_MODE_HIGH,
>         };
>         int i;
> 
>         for (i = 0; i < ARRAY_SIZE(snd_dice_rates); i++) {
>                 if (!(dice->clock_caps & BIT(i)))
>                         continue;
>                 if (snd_dice_rates[i] != rate)
>                         continue;
> 
>                 *mode = modes[i];
>                 return 0;
>         }
> 
>         return -EINVAL;
> }
> ```

That's one way, yes.
(BTW, you can write down in a style
	[0 ... 2] = SND_DICE_RATE_MODE_LOW,
too.)

Or, deal the exception explicity, e.g.

	*mode = i > 0 ? ((i - 1) / 2) : 0;

Or, mention just that i=0 shall give mode=0 with signed int.

I don't mind either way.


thanks,

Takashi

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

end of thread, other threads:[~2018-05-01  9:25 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-29  6:50 [PATCH v2 00/13] ALSA: dice: use cache of stream formats to generate PCM rules/constraints Takashi Sakamoto
2018-04-29  6:50 ` [PATCH v2 01/13] ALSA: dice: add cache of stream formats Takashi Sakamoto
2018-04-29  6:50 ` [PATCH v2 02/13] ALSA: dice: add 'firewire' directory for proc nodes Takashi Sakamoto
2018-04-29  6:50 ` [PATCH v2 03/13] ALSA: dice: add proc node for stream formation Takashi Sakamoto
2018-04-30 15:12   ` Takashi Iwai
2018-05-01  3:06     ` Takashi Sakamoto
2018-04-29  6:50 ` [PATCH v2 04/13] ALSA: dice: cache stream formats at current mode of sampling transmission frequency Takashi Sakamoto
2018-04-30 15:10   ` Takashi Iwai
2018-05-01  3:02     ` Takashi Sakamoto
2018-05-01  6:54       ` Takashi Iwai
2018-05-01  9:14         ` Takashi Sakamoto
2018-05-01  9:25           ` Takashi Iwai
2018-04-29  6:50 ` [PATCH v2 05/13] ALSA: dice: add parameters of stream formats for models produced by TC Electronic Takashi Sakamoto
2018-04-29  6:50 ` [PATCH v2 06/13] ALSA: dice: add parameters of stream formats for models produced by Alesis Takashi Sakamoto
2018-04-29  6:50 ` [PATCH v2 07/13] ALSA: dice: use extended protocol to detect available stream formats Takashi Sakamoto
2018-04-29  6:50 ` [PATCH v2 08/13] ALSA: dice: use cache of stream format to check running stream Takashi Sakamoto
2018-04-29  6:50 ` [PATCH v2 09/13] ALSA: dice: add a helper function to restart all of available streams Takashi Sakamoto
2018-04-29  6:50 ` [PATCH v2 10/13] ALSA: dice: enable to change current sampling transmission frequency Takashi Sakamoto
2018-04-29  6:50 ` [PATCH v2 11/13] ALSA: dice: use stream formats to add MIDI substreams Takashi Sakamoto
2018-04-29  6:50 ` [PATCH v2 12/13] ALSA: dice: use cache for PCM constraints and rules Takashi Sakamoto
2018-04-29  6:50 ` [PATCH v2 13/13] ALSA: dice: remove local frag of force_two_pcms Takashi Sakamoto

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.