[PATCH 0/6 v2] ALSA: dice: constrain PCM substreams to current sampling transfer frequency

[PATCH 0/6 v2] ALSA: dice: constrain PCM substreams to current sampling transfer frequency

[Takashi Sakamoto]

Hi,

This patchset is to update my former RFCv2 and for merge request to
upstream.

[alsa-devel][RFC][PATCH 00/10 v2] ALSA: dice: stabiliza packet streaming
http://mailman.alsa-project.org/pipermail/alsa-devel/2015-December/101897.html

Current ALSA Dice driver is written with somewhat ignoring actual hardware
design to perform 'dynamic sample rate selection'. As a result, the driver
includes some behaviors against users' expectation or functional
misleading.

This patchset purges such over-engineering. As a result, userspace
applications can start PCM substreams just at current sampling
transfer frequency. If users want to start at different sampling
rate, they must set favorite rate in advance by tools. Currently,
ffado-mixer with ffado-dbus-server[1] and hinawa-dice-common-cui[2] are
available as such tool.

For technical details, please refer to the post or our discussion in RFCv1:

[alsa-devel] [RFC][PATCH 0/8] ALSA: dice: constrain PCM substreams to current sampling transfer frequency
http://mailman.alsa-project.org/pipermail/alsa-devel/2015-November/100525.html

[1] as a part of FFADO utility
http://subversion.ffado.org/
[2] in hinawa-utils repository
https://github.com/takaswie/hinawa-utils


Regards

Takashi Sakamoto (6):
  ALSA: dice: limit to current sampling transfer frequency
  ALSA: dice: limit stream to current sampling transfer frequency.
  ALSA: dice: add MIDI ports according to current number of MIDI
    substreams
  ALSA: dice: get the number of MBLA data channel at opening PCM
    substream
  ALSA: dice: purge generating channel cache
  ALSA: dice: ensure phase lock before starting streaming

 sound/firewire/dice/dice-midi.c        |  23 +++-
 sound/firewire/dice/dice-pcm.c         | 199 +++++++++------------------------
 sound/firewire/dice/dice-stream.c      |  69 +++++++-----
 sound/firewire/dice/dice-transaction.c |  54 ---------
 sound/firewire/dice/dice.c             |  67 +----------
 sound/firewire/dice/dice.h             |   8 --
 6 files changed, 116 insertions(+), 304 deletions(-)

-- 
2.5.0

[PATCH 1/6] ALSA: dice: limit to current sampling transfer frequency

[Takashi Sakamoto]

ALSA PCM core has a functionality for rule of PCM substream parameters.
Typically, when userspace opens PCM character devices, each driver adds
its own rules to PCM substream according to design of hardware. When the
userspace executes hw_params ioctl with favorite parameters, the actual
parameters are calculated according to the rules and the given parameters.
Then, the result is returned to userspace.

Currently, ALSA Dice driver has the rule between channels and rates, while
Dice interface design doesn't allow drivers to retrieve all of the
combinations. Dice drivers are just allowed to get current sampling
transfer frequency and the number of multi bit linear audio data channels
in an data block of an AMDTP packet.

This commit purges the rule, and limit PCM substreams to current sampling
transfer frequency, following to the interface design.

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

diff --git a/sound/firewire/dice/dice-pcm.c b/sound/firewire/dice/dice-pcm.c
index 9b34319..0b6e6bb 100644
--- a/sound/firewire/dice/dice-pcm.c
+++ b/sound/firewire/dice/dice-pcm.c
@@ -9,99 +9,40 @@
 
 #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;
-
-	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 i, rate, mode, *pcm_channels;
-
-	if (substream->stream == SNDRV_PCM_STREAM_CAPTURE)
-		pcm_channels = dice->tx_channels;
-	else
-		pcm_channels = dice->rx_channels;
-
-	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;
-
-	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 i, rate, mode, *pcm_channels;
-
-	if (substream->stream == SNDRV_PCM_STREAM_CAPTURE)
-		pcm_channels = dice->tx_channels;
-	else
-		pcm_channels = dice->rx_channels;
-
-	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 void limit_channels_and_rates(struct snd_dice *dice,
-				     struct snd_pcm_runtime *runtime,
-				     unsigned int *pcm_channels)
+static int limit_channels_and_rates(struct snd_dice *dice,
+				    struct snd_pcm_runtime *runtime,
+				    struct amdtp_stream *stream)
 {
 	struct snd_pcm_hardware *hw = &runtime->hw;
-	unsigned int i, rate, mode;
+	unsigned int rate;
+	__be32 reg[2];
+	int err;
 
-	hw->channels_min = UINT_MAX;
-	hw->channels_max = 0;
+	/*
+	 * Retrieve current Multi Bit Linear Audio data channel and limit to
+	 * it.
+	 */
+	if (stream == &dice->tx_stream) {
+		err = snd_dice_transaction_read_tx(dice, TX_NUMBER_AUDIO,
+						   reg, sizeof(reg));
+	} else {
+		err = snd_dice_transaction_read_rx(dice, RX_NUMBER_AUDIO,
+						   reg, sizeof(reg));
+	}
+	if (err < 0)
+		return err;
 
-	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;
-		hw->rates |= snd_pcm_rate_to_rate_bit(rate);
+	hw->channels_min = hw->channels_max = be32_to_cpu(reg[0]);
 
-		if (pcm_channels[mode] == 0)
-			continue;
-		hw->channels_min = min(hw->channels_min, pcm_channels[mode]);
-		hw->channels_max = max(hw->channels_max, pcm_channels[mode]);
-	}
+	/* 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;
 }
 
 static void limit_period_and_buffer(struct snd_pcm_hardware *hw)
@@ -122,7 +63,6 @@ static int init_hw_info(struct snd_dice *dice,
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	struct snd_pcm_hardware *hw = &runtime->hw;
 	struct amdtp_stream *stream;
-	unsigned int *pcm_channels;
 	int err;
 
 	hw->info = SNDRV_PCM_INFO_MMAP |
@@ -135,37 +75,22 @@ static int init_hw_info(struct snd_dice *dice,
 	if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) {
 		hw->formats = AM824_IN_PCM_FORMAT_BITS;
 		stream = &dice->tx_stream;
-		pcm_channels = dice->tx_channels;
 	} else {
 		hw->formats = AM824_OUT_PCM_FORMAT_BITS;
 		stream = &dice->rx_stream;
-		pcm_channels = dice->rx_channels;
 	}
 
-	limit_channels_and_rates(dice, runtime, pcm_channels);
-	limit_period_and_buffer(hw);
-
-	err = snd_pcm_hw_rule_add(runtime, 0, SNDRV_PCM_HW_PARAM_RATE,
-				  dice_rate_constraint, substream,
-				  SNDRV_PCM_HW_PARAM_CHANNELS, -1);
+	err = limit_channels_and_rates(dice, runtime, stream);
 	if (err < 0)
-		goto end;
-	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)
-		goto end;
+		return err;
+	limit_period_and_buffer(hw);
 
-	err = amdtp_am824_add_pcm_hw_constraints(stream, runtime);
-end:
-	return err;
+	return amdtp_am824_add_pcm_hw_constraints(stream, runtime);
 }
 
 static int pcm_open(struct snd_pcm_substream *substream)
 {
 	struct snd_dice *dice = substream->private_data;
-	unsigned int source, rate;
-	bool internal;
 	int err;
 
 	err = snd_dice_stream_lock_try(dice);
@@ -176,39 +101,6 @@ 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) ||
-	    amdtp_stream_pcm_running(&dice->rx_stream)) {
-		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;
-- 
2.5.0

[PATCH 2/6] ALSA: dice: limit stream to current sampling transfer frequency.

[Takashi Sakamoto]

In previous commit, ALSA Dice driver limits PCM substreams at current
sampling transfer frequency and current number of Multi bit linear audio
data channel. Thus, the driver has no need to start AMDTP streams at
the other sampling transfer frequency except for current one. This is due
to Dice interface design.

This commit limits AMDTP stream at current sampling transfer frequency,
according to the design.

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

diff --git a/sound/firewire/dice/dice-stream.c b/sound/firewire/dice/dice-stream.c
index a6a39f7..4f74e3e 100644
--- a/sound/firewire/dice/dice-stream.c
+++ b/sound/firewire/dice/dice-stream.c
@@ -99,6 +99,7 @@ static int start_stream(struct snd_dice *dice, struct amdtp_stream *stream,
 			unsigned int rate)
 {
 	struct fw_iso_resources *resources;
+	__be32 reg[2];
 	unsigned int i, mode, pcm_chs, midi_ports;
 	bool double_pcm_frames;
 	int err;
@@ -108,14 +109,20 @@ static int start_stream(struct snd_dice *dice, struct amdtp_stream *stream,
 		goto end;
 	if (stream == &dice->tx_stream) {
 		resources = &dice->tx_resources;
-		pcm_chs = dice->tx_channels[mode];
-		midi_ports = dice->tx_midi_ports[mode];
+		err = snd_dice_transaction_read_tx(dice, TX_NUMBER_AUDIO,
+						   reg, sizeof(reg));
 	} else {
 		resources = &dice->rx_resources;
-		pcm_chs = dice->rx_channels[mode];
-		midi_ports = dice->rx_midi_ports[mode];
+		err = snd_dice_transaction_read_rx(dice, RX_NUMBER_AUDIO,
+						   reg, sizeof(reg));
 	}
 
+	if (err < 0)
+		goto end;
+
+	pcm_chs = be32_to_cpu(reg[0]);
+	midi_ports = be32_to_cpu(reg[1]);
+
 	/*
 	 * At 176.4/192.0 kHz, Dice has a quirk to transfer two PCM frames in
 	 * one data block of AMDTP packet. Thus sampling transfer frequency is
@@ -224,8 +231,10 @@ int snd_dice_stream_start_duplex(struct snd_dice *dice, unsigned int rate)
 	}
 	if (rate == 0)
 		rate = curr_rate;
-	if (rate != curr_rate)
-		stop_stream(dice, master);
+	if (rate != curr_rate) {
+		err = -EINVAL;
+		goto end;
+	}
 
 	if (!amdtp_stream_running(master)) {
 		stop_stream(dice, slave);
-- 
2.5.0

[PATCH 3/6] ALSA: dice: add MIDI ports according to current number of MIDI substreams

[Takashi Sakamoto]

This commit changes the way to add ALSA MIDI ports. This driver read the
number of multiplexed MIDI substreams from hardware register, then adds the
same number of ALSA MIDI ports. This commit is based on my assumption that
the number is fixed at all of supported sampling transfer frequency.

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

diff --git a/sound/firewire/dice/dice-midi.c b/sound/firewire/dice/dice-midi.c
index 151b09f..2461311 100644
--- a/sound/firewire/dice/dice-midi.c
+++ b/sound/firewire/dice/dice-midi.c
@@ -103,16 +103,27 @@ static void set_midi_substream_names(struct snd_dice *dice,
 
 int snd_dice_create_midi(struct snd_dice *dice)
 {
+	__be32 reg;
 	struct snd_rawmidi *rmidi;
 	struct snd_rawmidi_str *str;
-	unsigned int i, midi_in_ports, midi_out_ports;
+	unsigned int midi_in_ports, midi_out_ports;
 	int err;
 
-	midi_in_ports = midi_out_ports = 0;
-	for (i = 0; i < 3; i++) {
-		midi_in_ports = max(dice->tx_midi_ports[i], midi_in_ports);
-		midi_out_ports = max(dice->rx_midi_ports[i], midi_out_ports);
-	}
+	/*
+	 * 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);
 
 	if (midi_in_ports + midi_out_ports == 0)
 		return 0;
-- 
2.5.0

[PATCH 4/6] ALSA: dice: get the number of MBLA data channel at opening PCM substream

[Takashi Sakamoto]

This commit is a preparation to remove members related to channel cache
for the number of channels for multi bit linear audio data and MIDI
ports. This commit changes the way to get the number of multi bit linear
audio data channel. It's directly retrieved by asynchronous transactions
to some registers.

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

diff --git a/sound/firewire/dice/dice-pcm.c b/sound/firewire/dice/dice-pcm.c
index 0b6e6bb..a5c9b58 100644
--- a/sound/firewire/dice/dice-pcm.c
+++ b/sound/firewire/dice/dice-pcm.c
@@ -294,17 +294,30 @@ 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, capture, playback;
+	unsigned int capture, playback;
 	int err;
 
-	capture = playback = 0;
-	for (i = 0; i < 3; i++) {
-		if (dice->tx_channels[i] > 0)
-			capture = 1;
-		if (dice->rx_channels[i] > 0)
-			playback = 1;
-	}
+	/*
+	 * Check whether PCM substreams are required.
+	 *
+	 * TODO: in the case that any PCM substreams are not avail at a certain
+	 * sampling transfer frequency?
+	 */
+	err = snd_dice_transaction_read_tx(dice, TX_NUMBER_AUDIO,
+					   &reg, sizeof(reg));
+	if (err < 0)
+		return err;
+	if (be32_to_cpu(reg) > 0)
+		capture = 1;
+
+	err = snd_dice_transaction_read_rx(dice, RX_NUMBER_AUDIO,
+					   &reg, sizeof(reg));
+	if (err < 0)
+		return err;
+	if (be32_to_cpu(reg) > 0)
+		playback = 1;
 
 	err = snd_pcm_new(dice->card, "DICE", 0, playback, capture, &pcm);
 	if (err < 0)
-- 
2.5.0

[PATCH 5/6] ALSA: dice: purge generating channel cache

[Takashi Sakamoto]

Dice interface design doesn't allow drivers to read supported combination
between sampling transfer frequencies and the number of Multi bit linear
audio data channels. Due to the design, ALSA dice driver changes current
sampling transfer frequency to generate cache of the combinations at
device probing processing.

Although, this idea is worse because ALSA dice driver changes the state of
clock. This is not what users want when they save favorite configuration
to the device in advance.

Furthermore, there's a possibility that the format of data block is decided
not only according to current sampling transfer frequency, but also the
other factors, i.e. data format for digital interface. It's not good to
generate channel cache according to the sampling transfer frequency only.

This commit purges processing cache data and related structure members. As
a result, users must set preferable sampling transfer frequency before
using ALSA PCM applications, as long as they want to start any PCM
substreams at the rate except for current one.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/dice/dice-stream.c | 24 ++------------
 sound/firewire/dice/dice.c        | 67 ++-------------------------------------
 sound/firewire/dice/dice.h        |  7 ----
 3 files changed, 5 insertions(+), 93 deletions(-)

diff --git a/sound/firewire/dice/dice-stream.c b/sound/firewire/dice/dice-stream.c
index 4f74e3e..716db09 100644
--- a/sound/firewire/dice/dice-stream.c
+++ b/sound/firewire/dice/dice-stream.c
@@ -24,23 +24,6 @@ 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;
-}
-
 static void release_resources(struct snd_dice *dice,
 			      struct fw_iso_resources *resources)
 {
@@ -100,13 +83,10 @@ static int start_stream(struct snd_dice *dice, struct amdtp_stream *stream,
 {
 	struct fw_iso_resources *resources;
 	__be32 reg[2];
-	unsigned int i, mode, pcm_chs, midi_ports;
+	unsigned int i, pcm_chs, midi_ports;
 	bool double_pcm_frames;
 	int err;
 
-	err = snd_dice_stream_get_rate_mode(dice, rate, &mode);
-	if (err < 0)
-		goto end;
 	if (stream == &dice->tx_stream) {
 		resources = &dice->tx_resources;
 		err = snd_dice_transaction_read_tx(dice, TX_NUMBER_AUDIO,
@@ -133,7 +113,7 @@ static int start_stream(struct snd_dice *dice, struct amdtp_stream *stream,
 	 * For this quirk, blocking mode is required and PCM buffer size should
 	 * be aligned to SYT_INTERVAL.
 	 */
-	double_pcm_frames = mode > 1;
+	double_pcm_frames = rate > 96000;
 	if (double_pcm_frames) {
 		rate /= 2;
 		pcm_chs *= 2;
diff --git a/sound/firewire/dice/dice.c b/sound/firewire/dice/dice.c
index b91b373..f7303a6 100644
--- a/sound/firewire/dice/dice.c
+++ b/sound/firewire/dice/dice.c
@@ -57,65 +57,10 @@ static int check_dice_category(struct fw_unit *unit)
 	return 0;
 }
 
-static int highest_supported_mode_rate(struct snd_dice *dice,
-				       unsigned int mode, unsigned int *rate)
-{
-	unsigned int i, m;
-
-	for (i = ARRAY_SIZE(snd_dice_rates); i > 0; i--) {
-		*rate = snd_dice_rates[i - 1];
-		if (snd_dice_stream_get_rate_mode(dice, *rate, &m) < 0)
-			continue;
-		if (mode == m)
-			break;
-	}
-	if (i == 0)
-		return -EINVAL;
-
-	return 0;
-}
-
-static int dice_read_mode_params(struct snd_dice *dice, unsigned int mode)
-{
-	__be32 values[2];
-	unsigned int rate;
-	int err;
-
-	if (highest_supported_mode_rate(dice, mode, &rate) < 0) {
-		dice->tx_channels[mode] = 0;
-		dice->tx_midi_ports[mode] = 0;
-		dice->rx_channels[mode] = 0;
-		dice->rx_midi_ports[mode] = 0;
-		return 0;
-	}
-
-	err = snd_dice_transaction_set_rate(dice, rate);
-	if (err < 0)
-		return err;
-
-	err = snd_dice_transaction_read_tx(dice, TX_NUMBER_AUDIO,
-					   values, sizeof(values));
-	if (err < 0)
-		return err;
-
-	dice->tx_channels[mode]   = be32_to_cpu(values[0]);
-	dice->tx_midi_ports[mode] = be32_to_cpu(values[1]);
-
-	err = snd_dice_transaction_read_rx(dice, RX_NUMBER_AUDIO,
-					   values, sizeof(values));
-	if (err < 0)
-		return err;
-
-	dice->rx_channels[mode]   = be32_to_cpu(values[0]);
-	dice->rx_midi_ports[mode] = be32_to_cpu(values[1]);
-
-	return 0;
-}
-
-static int dice_read_params(struct snd_dice *dice)
+static int check_clock_caps(struct snd_dice *dice)
 {
 	__be32 value;
-	int mode, err;
+	int err;
 
 	/* some very old firmwares don't tell about their clock support */
 	if (dice->clock_caps > 0) {
@@ -133,12 +78,6 @@ static int dice_read_params(struct snd_dice *dice)
 				   CLOCK_CAP_SOURCE_INTERNAL;
 	}
 
-	for (mode = 2; mode >= 0; --mode) {
-		err = dice_read_mode_params(dice, mode);
-		if (err < 0)
-			return err;
-	}
-
 	return 0;
 }
 
@@ -215,7 +154,7 @@ static void do_registration(struct work_struct *work)
 	if (err < 0)
 		goto error;
 
-	err = dice_read_params(dice);
+	err = check_clock_caps(dice);
 	if (err < 0)
 		goto error;
 
diff --git a/sound/firewire/dice/dice.h b/sound/firewire/dice/dice.h
index 3d5ebeb..c968f98 100644
--- a/sound/firewire/dice/dice.h
+++ b/sound/firewire/dice/dice.h
@@ -56,10 +56,6 @@ struct snd_dice {
 	unsigned int rsrv_offset;
 
 	unsigned int clock_caps;
-	unsigned int tx_channels[3];
-	unsigned int rx_channels[3];
-	unsigned int tx_midi_ports[3];
-	unsigned int rx_midi_ports[3];
 
 	struct fw_address_handler notification_handler;
 	int owner_generation;
@@ -169,9 +165,6 @@ 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);
-- 
2.5.0

[PATCH 6/6] ALSA: dice: ensure phase lock before starting streaming

[Takashi Sakamoto]

In former commits, probing process has no need to set sampling transfer
frequency. Although it's OK to drop a function to change the frequency
from this module, some models require it before streaming. This seems to
be due to phase lock of clock source.

This commit moves the function from transaction layer to stream layer, and
rename it according to the purpose.

Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/dice/dice-stream.c      | 34 +++++++++++++++++++--
 sound/firewire/dice/dice-transaction.c | 54 ----------------------------------
 sound/firewire/dice/dice.h             |  1 -
 3 files changed, 32 insertions(+), 57 deletions(-)

diff --git a/sound/firewire/dice/dice-stream.c b/sound/firewire/dice/dice-stream.c
index 716db09..e4938b0 100644
--- a/sound/firewire/dice/dice-stream.c
+++ b/sound/firewire/dice/dice-stream.c
@@ -10,6 +10,7 @@
 #include "dice.h"
 
 #define	CALLBACK_TIMEOUT	200
+#define NOTIFICATION_TIMEOUT_MS	(2 * MSEC_PER_SEC)
 
 const unsigned int snd_dice_rates[SND_DICE_RATES_COUNT] = {
 	/* mode 0 */
@@ -24,6 +25,35 @@ const unsigned int snd_dice_rates[SND_DICE_RATES_COUNT] = {
 	[6] = 192000,
 };
 
+/*
+ * 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)
+{
+	__be32 reg;
+	int err;
+
+	err = snd_dice_transaction_read_global(dice, GLOBAL_CLOCK_SELECT,
+					       &reg, sizeof(reg));
+	if (err < 0)
+		return err;
+
+	if (completion_done(&dice->clock_accepted))
+		reinit_completion(&dice->clock_accepted);
+
+	err = snd_dice_transaction_write_global(dice, GLOBAL_CLOCK_SELECT,
+						&reg, sizeof(reg));
+	if (err < 0)
+		return err;
+
+	if (wait_for_completion_timeout(&dice->clock_accepted,
+			msecs_to_jiffies(NOTIFICATION_TIMEOUT_MS)) == 0)
+		return -ETIMEDOUT;
+
+	return 0;
+}
+
 static void release_resources(struct snd_dice *dice,
 			      struct fw_iso_resources *resources)
 {
@@ -222,10 +252,10 @@ int snd_dice_stream_start_duplex(struct snd_dice *dice, unsigned int rate)
 
 		amdtp_stream_set_sync(sync_mode, master, slave);
 
-		err = snd_dice_transaction_set_rate(dice, rate);
+		err = ensure_phase_lock(dice);
 		if (err < 0) {
 			dev_err(&dice->unit->device,
-				"fail to set sampling rate\n");
+				"fail to ensure phase lock\n");
 			goto end;
 		}
 
diff --git a/sound/firewire/dice/dice-transaction.c b/sound/firewire/dice/dice-transaction.c
index a4ff4e0..76f9f72 100644
--- a/sound/firewire/dice/dice-transaction.c
+++ b/sound/firewire/dice/dice-transaction.c
@@ -9,8 +9,6 @@
 
 #include "dice.h"
 
-#define NOTIFICATION_TIMEOUT_MS	(2 * MSEC_PER_SEC)
-
 static u64 get_subaddr(struct snd_dice *dice, enum snd_dice_addr_type type,
 		       u64 offset)
 {
@@ -62,54 +60,6 @@ static unsigned int get_clock_info(struct snd_dice *dice, __be32 *info)
 						info, 4);
 }
 
-static int set_clock_info(struct snd_dice *dice,
-			  unsigned int rate, unsigned int source)
-{
-	unsigned int i;
-	__be32 info;
-	u32 mask;
-	u32 clock;
-	int err;
-
-	err = get_clock_info(dice, &info);
-	if (err < 0)
-		return err;
-
-	clock = be32_to_cpu(info);
-	if (source != UINT_MAX) {
-		mask = CLOCK_SOURCE_MASK;
-		clock &= ~mask;
-		clock |= source;
-	}
-	if (rate != UINT_MAX) {
-		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;
-
-		mask = CLOCK_RATE_MASK;
-		clock &= ~mask;
-		clock |= i << CLOCK_RATE_SHIFT;
-	}
-	info = cpu_to_be32(clock);
-
-	if (completion_done(&dice->clock_accepted))
-		reinit_completion(&dice->clock_accepted);
-
-	err = snd_dice_transaction_write_global(dice, GLOBAL_CLOCK_SELECT,
-						&info, 4);
-	if (err < 0)
-		return err;
-
-	if (wait_for_completion_timeout(&dice->clock_accepted,
-			msecs_to_jiffies(NOTIFICATION_TIMEOUT_MS)) == 0)
-		return -ETIMEDOUT;
-
-	return 0;
-}
-
 int snd_dice_transaction_get_clock_source(struct snd_dice *dice,
 					  unsigned int *source)
 {
@@ -143,10 +93,6 @@ int snd_dice_transaction_get_rate(struct snd_dice *dice, unsigned int *rate)
 end:
 	return err;
 }
-int snd_dice_transaction_set_rate(struct snd_dice *dice, unsigned int rate)
-{
-	return set_clock_info(dice, rate, UINT_MAX);
-}
 
 int snd_dice_transaction_set_enable(struct snd_dice *dice)
 {
diff --git a/sound/firewire/dice/dice.h b/sound/firewire/dice/dice.h
index c968f98..423cdba 100644
--- a/sound/firewire/dice/dice.h
+++ b/sound/firewire/dice/dice.h
@@ -154,7 +154,6 @@ static inline int snd_dice_transaction_read_sync(struct snd_dice *dice,
 
 int snd_dice_transaction_get_clock_source(struct snd_dice *dice,
 					  unsigned int *source);
-int snd_dice_transaction_set_rate(struct snd_dice *dice, unsigned int rate);
 int snd_dice_transaction_get_rate(struct snd_dice *dice, unsigned int *rate);
 int snd_dice_transaction_set_enable(struct snd_dice *dice);
 void snd_dice_transaction_clear_enable(struct snd_dice *dice);
-- 
2.5.0

Re: [PATCH 0/6 v2] ALSA: dice: constrain PCM substreams to current sampling transfer frequency

[Takashi Iwai]

On Mon, 08 Feb 2016 14:54:14 +0100,
Takashi Sakamoto wrote:
> 
> Hi,
> 
> This patchset is to update my former RFCv2 and for merge request to
> upstream.
> 
> [alsa-devel][RFC][PATCH 00/10 v2] ALSA: dice: stabiliza packet streaming
> http://mailman.alsa-project.org/pipermail/alsa-devel/2015-December/101897.html
> 
> Current ALSA Dice driver is written with somewhat ignoring actual hardware
> design to perform 'dynamic sample rate selection'. As a result, the driver
> includes some behaviors against users' expectation or functional
> misleading.
> 
> This patchset purges such over-engineering. As a result, userspace
> applications can start PCM substreams just at current sampling
> transfer frequency. If users want to start at different sampling
> rate, they must set favorite rate in advance by tools. Currently,
> ffado-mixer with ffado-dbus-server[1] and hinawa-dice-common-cui[2] are
> available as such tool.
> 
> For technical details, please refer to the post or our discussion in RFCv1:
> 
> [alsa-devel] [RFC][PATCH 0/8] ALSA: dice: constrain PCM substreams to current sampling transfer frequency
> http://mailman.alsa-project.org/pipermail/alsa-devel/2015-November/100525.html
> 
> [1] as a part of FFADO utility
> http://subversion.ffado.org/
> [2] in hinawa-utils repository
> https://github.com/takaswie/hinawa-utils
> 
> 
> Regards
> 
> Takashi Sakamoto (6):
>   ALSA: dice: limit to current sampling transfer frequency
>   ALSA: dice: limit stream to current sampling transfer frequency.
>   ALSA: dice: add MIDI ports according to current number of MIDI
>     substreams
>   ALSA: dice: get the number of MBLA data channel at opening PCM
>     substream
>   ALSA: dice: purge generating channel cache
>   ALSA: dice: ensure phase lock before starting streaming

Applied all six patches now to for-next branch.  Thanks.


Takashi

Re: [PATCH 0/6 v2] ALSA: dice: constrain PCM substreams to current sampling transfer frequency

[Takashi Sakamoto]

On Feb 9 2016 20:25, Takashi Iwai wrote:
> On Mon, 08 Feb 2016 14:54:14 +0100,
> Takashi Sakamoto wrote:
>>
>> Hi,
>>
>> This patchset is to update my former RFCv2 and for merge request to
>> upstream.
>>
>> [alsa-devel][RFC][PATCH 00/10 v2] ALSA: dice: stabiliza packet streaming
>> http://mailman.alsa-project.org/pipermail/alsa-devel/2015-December/101897.html
>>
>> Current ALSA Dice driver is written with somewhat ignoring actual hardware
>> design to perform 'dynamic sample rate selection'. As a result, the driver
>> includes some behaviors against users' expectation or functional
>> misleading.
>>
>> This patchset purges such over-engineering. As a result, userspace
>> applications can start PCM substreams just at current sampling
>> transfer frequency. If users want to start at different sampling
>> rate, they must set favorite rate in advance by tools. Currently,
>> ffado-mixer with ffado-dbus-server[1] and hinawa-dice-common-cui[2] are
>> available as such tool.
>>
>> For technical details, please refer to the post or our discussion in RFCv1:
>>
>> [alsa-devel] [RFC][PATCH 0/8] ALSA: dice: constrain PCM substreams to current sampling transfer frequency
>> http://mailman.alsa-project.org/pipermail/alsa-devel/2015-November/100525.html
>>
>> [1] as a part of FFADO utility
>> http://subversion.ffado.org/
>> [2] in hinawa-utils repository
>> https://github.com/takaswie/hinawa-utils
>>
>>
>> Regards
>>
>> Takashi Sakamoto (6):
>>   ALSA: dice: limit to current sampling transfer frequency
>>   ALSA: dice: limit stream to current sampling transfer frequency.
>>   ALSA: dice: add MIDI ports according to current number of MIDI
>>     substreams
>>   ALSA: dice: get the number of MBLA data channel at opening PCM
>>     substream
>>   ALSA: dice: purge generating channel cache
>>   ALSA: dice: ensure phase lock before starting streaming
> 
> Applied all six patches now to for-next branch.  Thanks.

I'm ease to hear the merge because I've been considering about this
issue for one and half year (since Sep 2014).


Thanks

Takashi Sakamoto