[alsa-devel] [PATCH 0/6 v2] ALSA: dice: constrain PCM substreams to current sampling transfer frequency
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.ht...
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.ht...
[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(-)
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;
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);
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, + ®, sizeof(reg)); + if (err < 0) + return err; + midi_in_ports = be32_to_cpu(reg); + + err = snd_dice_transaction_read_rx(dice, RX_NUMBER_MIDI, + ®, sizeof(reg)); + if (err < 0) + return err; + midi_out_ports = be32_to_cpu(reg);
if (midi_in_ports + midi_out_ports == 0) return 0;
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, + ®, 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, + ®, 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)
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);
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, + ®, 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, + ®, 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);
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.ht...
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.ht...
[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
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.ht...
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.ht...
[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
participants (2)
-
Takashi Iwai
-
Takashi Sakamoto