diff mbox series

ALSA: oxfw: fix functional regression for Mackie Onyx 1640i in v5.14 or later

Message ID 20211028130325.45772-1-o-takashi@sakamocchi.jp (mailing list archive)
State Accepted
Commit cddcd5472abb7b8a9c37ccbcf0908b79740a01b5
Headers show
Series ALSA: oxfw: fix functional regression for Mackie Onyx 1640i in v5.14 or later | expand

Commit Message

Takashi Sakamoto Oct. 28, 2021, 1:03 p.m. UTC
A user reports functional regression for Mackie Onyx 1640i that the device
generates slow sound with ALSA oxfw driver which supports media clock
recovery. Although the device is based on OXFW971 ASIC, it does not
transfer isochronous packet with own event frequency as expected. The
device seems to adjust event frequency according to events in received
isochronous packets in the beginning of packet streaming. This is
unknown quirk.

This commit fixes the regression to turn the recovery off in driver
side. As a result, nominal frequency is used in duplex packet streaming
between device and driver. For stability of sampling rate in events of
transferred isochronous packet, 4,000 isochronous packets are skipped
in the beginning of packet streaming.

Reference: https://github.com/takaswie/snd-firewire-improve/issues/38
Fixes: 029ffc429440 ("ALSA: oxfw: perform sequence replay for media clock recovery")
Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
---
 sound/firewire/oxfw/oxfw-stream.c | 7 ++++++-
 sound/firewire/oxfw/oxfw.c        | 8 ++++++++
 sound/firewire/oxfw/oxfw.h        | 5 +++++
 3 files changed, 19 insertions(+), 1 deletion(-)

Comments

Takashi Iwai Oct. 28, 2021, 3:44 p.m. UTC | #1
On Thu, 28 Oct 2021 15:03:25 +0200,
Takashi Sakamoto wrote:
> 
> A user reports functional regression for Mackie Onyx 1640i that the device
> generates slow sound with ALSA oxfw driver which supports media clock
> recovery. Although the device is based on OXFW971 ASIC, it does not
> transfer isochronous packet with own event frequency as expected. The
> device seems to adjust event frequency according to events in received
> isochronous packets in the beginning of packet streaming. This is
> unknown quirk.
> 
> This commit fixes the regression to turn the recovery off in driver
> side. As a result, nominal frequency is used in duplex packet streaming
> between device and driver. For stability of sampling rate in events of
> transferred isochronous packet, 4,000 isochronous packets are skipped
> in the beginning of packet streaming.
> 
> Reference: https://github.com/takaswie/snd-firewire-improve/issues/38
> Fixes: 029ffc429440 ("ALSA: oxfw: perform sequence replay for media clock recovery")
> Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>

Thanks, applied.


Takashi
Takashi Sakamoto Oct. 29, 2021, 12:30 a.m. UTC | #2
On Thu, Oct 28, 2021 at 05:44:37PM +0200, Takashi Iwai wrote:
> On Thu, 28 Oct 2021 15:03:25 +0200,
> Takashi Sakamoto wrote:
> > 
> > A user reports functional regression for Mackie Onyx 1640i that the device
> > generates slow sound with ALSA oxfw driver which supports media clock
> > recovery. Although the device is based on OXFW971 ASIC, it does not
> > transfer isochronous packet with own event frequency as expected. The
> > device seems to adjust event frequency according to events in received
> > isochronous packets in the beginning of packet streaming. This is
> > unknown quirk.
> > 
> > This commit fixes the regression to turn the recovery off in driver
> > side. As a result, nominal frequency is used in duplex packet streaming
> > between device and driver. For stability of sampling rate in events of
> > transferred isochronous packet, 4,000 isochronous packets are skipped
> > in the beginning of packet streaming.
> > 
> > Reference: https://github.com/takaswie/snd-firewire-improve/issues/38
> > Fixes: 029ffc429440 ("ALSA: oxfw: perform sequence replay for media clock recovery")
> > Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
> 
> Thanks, applied.
> 
> 
> Takashi

Oops. I forget to add tag to stable. It's preferable to apply the
patch to current stable (5.14.15) and mainline RC (5.15-rc7) as well...

Would I request you to post the patch for the purpose alternatively?
(It is perhaps too late for mainline, I guess...)


Regards

Takashi Sakamoto
Takashi Iwai Oct. 29, 2021, 7:49 a.m. UTC | #3
On Fri, 29 Oct 2021 02:30:22 +0200,
Takashi Sakamoto wrote:
> 
> On Thu, Oct 28, 2021 at 05:44:37PM +0200, Takashi Iwai wrote:
> > On Thu, 28 Oct 2021 15:03:25 +0200,
> > Takashi Sakamoto wrote:
> > > 
> > > A user reports functional regression for Mackie Onyx 1640i that the device
> > > generates slow sound with ALSA oxfw driver which supports media clock
> > > recovery. Although the device is based on OXFW971 ASIC, it does not
> > > transfer isochronous packet with own event frequency as expected. The
> > > device seems to adjust event frequency according to events in received
> > > isochronous packets in the beginning of packet streaming. This is
> > > unknown quirk.
> > > 
> > > This commit fixes the regression to turn the recovery off in driver
> > > side. As a result, nominal frequency is used in duplex packet streaming
> > > between device and driver. For stability of sampling rate in events of
> > > transferred isochronous packet, 4,000 isochronous packets are skipped
> > > in the beginning of packet streaming.
> > > 
> > > Reference: https://github.com/takaswie/snd-firewire-improve/issues/38
> > > Fixes: 029ffc429440 ("ALSA: oxfw: perform sequence replay for media clock recovery")
> > > Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
> > 
> > Thanks, applied.
> > 
> > 
> > Takashi
> 
> Oops. I forget to add tag to stable. It's preferable to apply the
> patch to current stable (5.14.15) and mainline RC (5.15-rc7) as well...
> 
> Would I request you to post the patch for the purpose alternatively?
> (It is perhaps too late for mainline, I guess...)

You can inform Greg and Sasha once after the commit gets merged to
Linus tree by yourself.  Just tell them the upstream commit id and
corresponding stable trees to merge.  Usually the stable AUTOSEL
mechanism will pick up such a commit automatically, but doing the
manual selection is more certain and faster.


thanks,

Takashi
Takashi Sakamoto Oct. 29, 2021, 8:35 a.m. UTC | #4
On Fri, Oct 29, 2021 at 09:49:35AM +0200, Takashi Iwai wrote:
> On Fri, 29 Oct 2021 02:30:22 +0200,
> Takashi Sakamoto wrote:
> > 
> > On Thu, Oct 28, 2021 at 05:44:37PM +0200, Takashi Iwai wrote:
> > > On Thu, 28 Oct 2021 15:03:25 +0200,
> > > Takashi Sakamoto wrote:
> > > > 
> > > > A user reports functional regression for Mackie Onyx 1640i that the device
> > > > generates slow sound with ALSA oxfw driver which supports media clock
> > > > recovery. Although the device is based on OXFW971 ASIC, it does not
> > > > transfer isochronous packet with own event frequency as expected. The
> > > > device seems to adjust event frequency according to events in received
> > > > isochronous packets in the beginning of packet streaming. This is
> > > > unknown quirk.
> > > > 
> > > > This commit fixes the regression to turn the recovery off in driver
> > > > side. As a result, nominal frequency is used in duplex packet streaming
> > > > between device and driver. For stability of sampling rate in events of
> > > > transferred isochronous packet, 4,000 isochronous packets are skipped
> > > > in the beginning of packet streaming.
> > > > 
> > > > Reference: https://github.com/takaswie/snd-firewire-improve/issues/38
> > > > Fixes: 029ffc429440 ("ALSA: oxfw: perform sequence replay for media clock recovery")
> > > > Signed-off-by: Takashi Sakamoto <o-takashi@sakamocchi.jp>
> > > 
> > > Thanks, applied.
> > > 
> > > 
> > > Takashi
> > 
> > Oops. I forget to add tag to stable. It's preferable to apply the
> > patch to current stable (5.14.15) and mainline RC (5.15-rc7) as well...
> > 
> > Would I request you to post the patch for the purpose alternatively?
> > (It is perhaps too late for mainline, I guess...)
> 
> You can inform Greg and Sasha once after the commit gets merged to
> Linus tree by yourself.  Just tell them the upstream commit id and
> corresponding stable trees to merge.  Usually the stable AUTOSEL
> mechanism will pick up such a commit automatically, but doing the
> manual selection is more certain and faster.

Thank you for the instruction. I've rarely used the request path for
the manual selection. Ok, when the stable AUTOSEL doesn't pick up the
commit enough later releasing mainline, I'll follow the path.


Thanks

Takashi Sakamoto
diff mbox series

Patch

diff --git a/sound/firewire/oxfw/oxfw-stream.c b/sound/firewire/oxfw/oxfw-stream.c
index fff18b5d4e05..f4a702def397 100644
--- a/sound/firewire/oxfw/oxfw-stream.c
+++ b/sound/firewire/oxfw/oxfw-stream.c
@@ -9,7 +9,7 @@ 
 #include <linux/delay.h>
 
 #define AVC_GENERIC_FRAME_MAXIMUM_BYTES	512
-#define READY_TIMEOUT_MS	200
+#define READY_TIMEOUT_MS	600
 
 /*
  * According to datasheet of Oxford Semiconductor:
@@ -367,6 +367,11 @@  int snd_oxfw_stream_start_duplex(struct snd_oxfw *oxfw)
 				// Just after changing sampling transfer frequency, many cycles are
 				// skipped for packet transmission.
 				tx_init_skip_cycles = 400;
+			} else if (oxfw->quirks & SND_OXFW_QUIRK_VOLUNTARY_RECOVERY) {
+				// It takes a bit time for target device to adjust event frequency
+				// according to nominal event frequency in isochronous packets from
+				// ALSA oxfw driver.
+				tx_init_skip_cycles = 4000;
 			} else {
 				replay_seq = true;
 			}
diff --git a/sound/firewire/oxfw/oxfw.c b/sound/firewire/oxfw/oxfw.c
index daf731364695..b496f87841ae 100644
--- a/sound/firewire/oxfw/oxfw.c
+++ b/sound/firewire/oxfw/oxfw.c
@@ -25,6 +25,7 @@ 
 #define MODEL_SATELLITE		0x00200f
 #define MODEL_SCS1M		0x001000
 #define MODEL_DUET_FW		0x01dddd
+#define MODEL_ONYX_1640I	0x001640
 
 #define SPECIFIER_1394TA	0x00a02d
 #define VERSION_AVC		0x010001
@@ -192,6 +193,13 @@  static int detect_quirks(struct snd_oxfw *oxfw, const struct ieee1394_device_id
 		// OXFW971-based models may transfer events by blocking method.
 		if (!(oxfw->quirks & SND_OXFW_QUIRK_JUMBO_PAYLOAD))
 			oxfw->quirks |= SND_OXFW_QUIRK_BLOCKING_TRANSMISSION;
+
+		if (model == MODEL_ONYX_1640I) {
+			//Unless receiving packets without NOINFO packet, the device transfers
+			//mostly half of events in packets than expected.
+			oxfw->quirks |= SND_OXFW_QUIRK_IGNORE_NO_INFO_PACKET |
+					SND_OXFW_QUIRK_VOLUNTARY_RECOVERY;
+		}
 	}
 
 	return 0;
diff --git a/sound/firewire/oxfw/oxfw.h b/sound/firewire/oxfw/oxfw.h
index c13034f6c2ca..d728e451a25c 100644
--- a/sound/firewire/oxfw/oxfw.h
+++ b/sound/firewire/oxfw/oxfw.h
@@ -47,6 +47,11 @@  enum snd_oxfw_quirk {
 	// the device to process audio data even if the value is invalid in a point of
 	// IEC 61883-1/6.
 	SND_OXFW_QUIRK_IGNORE_NO_INFO_PACKET = 0x10,
+	// Loud Technologies Mackie Onyx 1640i seems to configure OXFW971 ASIC so that it decides
+	// event frequency according to events in received isochronous packets. The device looks to
+	// performs media clock recovery voluntarily. In the recovery, the packets with NO_INFO
+	// are ignored, thus driver should transfer packets with timestamp.
+	SND_OXFW_QUIRK_VOLUNTARY_RECOVERY = 0x20,
 };
 
 /* This is an arbitrary number for convinience. */