[PATCH 0/2] Fix a regression for Dice driver at higher sampling rate

[PATCH 0/2] Fix a regression for Dice driver at higher sampling rate

[Takashi Sakamoto]

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(-)

-- 
1.9.1

[PATCH 1/2] ALSA: dice: fix wrong channel mappping at higher sampling rate

[Takashi Sakamoto]

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));
 
-- 
1.9.1

[PATCH 2/2] ALSA: firewire-lib/dice: add arrangements of PCM pointer and interrupts for Dice quirk

[Takashi Sakamoto]

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);
-- 
1.9.1

Re: [PATCH 0/2] Fix a regression for Dice driver at higher sampling rate

[Takashi Iwai]

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
>