[alsa-devel] [PATCH 0/2] Fix a regression for Dice driver at higher sampling rate
Commit 10550bea44a8 ("ALSA: dice/firewire-lib: Keep dualwire mode but obsolete CIP_HI_DUALWIRE") brought a regression to Dice driver. As a result, Dice driver cannot work correctly at higher sampling rate.
This series of patches fixes the regression.
Reported-by: Daniel Robbins drobbins@funtoo.org Fixes: 10550bea44a8 ("ALSA: dice/firewire-lib: Keep dualwire mode but obsolete CIP_HI_DUALWIRE")
Takashi Sakamoto (2): ALSA: dice: fix wrong channel mappping at higher sampling rate ALSA: firewire-lib/dice: add arrangements of PCM pointer and interrupts for Dice quirk
sound/firewire/amdtp.c | 11 ++++++++++- sound/firewire/amdtp.h | 1 + sound/firewire/dice.c | 29 ++++++++++++++++++++--------- 3 files changed, 31 insertions(+), 10 deletions(-)
The channel mapping is initialized by amdtp_stream_set_parameters(), however Dice driver set it before calling this function. Furthermore, the setting is wrong because the index is the value of array, and vice versa.
This commit moves codes for channel mapping after the function and set it correctly.
Reported-by: Daniel Robbins drobbins@funtoo.org Fixes: 10550bea44a8 ("ALSA: dice/firewire-lib: Keep dualwire mode but obsolete CIP_HI_DUALWIRE") Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/firewire/dice.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)
diff --git a/sound/firewire/dice.c b/sound/firewire/dice.c index a9a30c0..4cf8eb7 100644 --- a/sound/firewire/dice.c +++ b/sound/firewire/dice.c @@ -579,11 +579,6 @@ static int dice_hw_params(struct snd_pcm_substream *substream, return err; }
- for (i = 0; i < channels; i++) { - dice->stream.pcm_positions[i * 2] = i; - dice->stream.pcm_positions[i * 2 + 1] = i + channels; - } - rate /= 2; channels *= 2; } @@ -591,6 +586,15 @@ static int dice_hw_params(struct snd_pcm_substream *substream, mode = rate_index_to_mode(rate_index); amdtp_stream_set_parameters(&dice->stream, rate, channels, dice->rx_midi_ports[mode]); + if (rate_index > 4) { + channels /= 2; + + for (i = 0; i < channels; i++) { + dice->stream.pcm_positions[i] = i * 2; + dice->stream.pcm_positions[i + channels] = i * 2 + 1; + } + } + amdtp_stream_set_pcm_format(&dice->stream, params_format(hw_params));
In IEC 61883-6, one data block transfers one event. In ALSA, the event equals one PCM frame, hence one data block transfers one PCM frame. But Dice has a quirk at higher sampling rate (176.4/192.0 kHz) that one data block transfers two PCM frames.
Commit 10550bea44a8 ("ALSA: dice/firewire-lib: Keep dualwire mode but obsolete CIP_HI_DUALWIRE") moved some codes related to this quirk into Dice driver. But the commit forgot to add arrangements for PCM period interrupts and DMA pointer updates. As a result, Dice driver cannot work correctly at higher sampling rate.
This commit adds 'double_pcm_frames' parameter to amdtp structure for this quirk. When this parameter is set, PCM period interrupts and DMA pointer updates occur at double speed than in IEC 61883-6.
Reported-by: Daniel Robbins drobbins@funtoo.org Fixes: 10550bea44a8 ("ALSA: dice/firewire-lib: Keep dualwire mode but obsolete CIP_HI_DUALWIRE") Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/firewire/amdtp.c | 11 ++++++++++- sound/firewire/amdtp.h | 1 + sound/firewire/dice.c | 15 +++++++++++---- 3 files changed, 22 insertions(+), 5 deletions(-)
diff --git a/sound/firewire/amdtp.c b/sound/firewire/amdtp.c index f96bf4c..95fc2eaf 100644 --- a/sound/firewire/amdtp.c +++ b/sound/firewire/amdtp.c @@ -507,7 +507,16 @@ static void amdtp_pull_midi(struct amdtp_stream *s, static void update_pcm_pointers(struct amdtp_stream *s, struct snd_pcm_substream *pcm, unsigned int frames) -{ unsigned int ptr; +{ + unsigned int ptr; + + /* + * In IEC 61883-6, one data block represents one event. In ALSA, one + * event equals to one PCM frame. But Dice has a quirk to transfer + * two PCM frames in one data block. + */ + if (s->double_pcm_frames) + frames *= 2;
ptr = s->pcm_buffer_pointer + frames; if (ptr >= pcm->runtime->buffer_size) diff --git a/sound/firewire/amdtp.h b/sound/firewire/amdtp.h index d8ee7b0..4823c08 100644 --- a/sound/firewire/amdtp.h +++ b/sound/firewire/amdtp.h @@ -125,6 +125,7 @@ struct amdtp_stream { unsigned int pcm_buffer_pointer; unsigned int pcm_period_pointer; bool pointer_flush; + bool double_pcm_frames;
struct snd_rawmidi_substream *midi[AMDTP_MAX_CHANNELS_FOR_MIDI * 8];
diff --git a/sound/firewire/dice.c b/sound/firewire/dice.c index 4cf8eb7..e3a04d6 100644 --- a/sound/firewire/dice.c +++ b/sound/firewire/dice.c @@ -567,10 +567,14 @@ static int dice_hw_params(struct snd_pcm_substream *substream, return err;
/* - * At rates above 96 kHz, pretend that the stream runs at half the - * actual sample rate with twice the number of channels; two samples - * of a channel are stored consecutively in the packet. Requires - * blocking mode and PCM buffer size should be aligned to SYT_INTERVAL. + * 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 + * a half of PCM sampling frequency, i.e. PCM frames at 192.0 kHz are + * transferred on AMDTP packets at 96 kHz. Two successive samples of a + * channel are stored consecutively in the packet. This quirk is called + * as 'Dual Wire'. + * For this quirk, blocking mode is required and PCM buffer size should + * be aligned to SYT_INTERVAL. */ channels = params_channels(hw_params); if (rate_index > 4) { @@ -581,6 +585,9 @@ static int dice_hw_params(struct snd_pcm_substream *substream,
rate /= 2; channels *= 2; + dice->stream.double_pcm_frames = true; + } else { + dice->stream.double_pcm_frames = false; }
mode = rate_index_to_mode(rate_index);
At Fri, 29 Aug 2014 13:40:43 +0900, Takashi Sakamoto wrote:
Commit 10550bea44a8 ("ALSA: dice/firewire-lib: Keep dualwire mode but obsolete CIP_HI_DUALWIRE") brought a regression to Dice driver. As a result, Dice driver cannot work correctly at higher sampling rate.
This series of patches fixes the regression.
Reported-by: Daniel Robbins drobbins@funtoo.org Fixes: 10550bea44a8 ("ALSA: dice/firewire-lib: Keep dualwire mode but obsolete CIP_HI_DUALWIRE")
Takashi Sakamoto (2): ALSA: dice: fix wrong channel mappping at higher sampling rate ALSA: firewire-lib/dice: add arrangements of PCM pointer and interrupts for Dice quirk
Applied both patches now (with Cc to stable). Thanks.
Takashi
sound/firewire/amdtp.c | 11 ++++++++++- sound/firewire/amdtp.h | 1 + sound/firewire/dice.c | 29 ++++++++++++++++++++--------- 3 files changed, 31 insertions(+), 10 deletions(-)
-- 1.9.1
participants (2)
-
Takashi Iwai -
Takashi Sakamoto