diff mbox series

[RFC] ALSA: Add a static keep_iface quirk for two known devices

Message ID 20180830195717.29474-1-news@aboehler.at (mailing list archive)
State New, archived
Headers show
Series [RFC] ALSA: Add a static keep_iface quirk for two known devices | expand

Commit Message

news@aboehler.at Aug. 30, 2018, 7:57 p.m. UTC 
From: Andreas Böhler <news@aboehler.at>

Hi,

I own a Phonic Helix Board 12 Universal USB sound mixer that I couldn't 
get to work properly on my box. After some debugging, it turned out that 
the playback stream is muted when the capture interface is brought down.

Since I was on 4.14, I implemented a quirk there and wanted to port it 
to 4.18 before submitting it - just to find out that the keep_iface 
control is very much the same approach.

This patch adds a mixer quirk for known devices such as the Phonic Helix 
Board 12 Universal and the Behringer FCA610 (tested and reported by 
Takashi Sakamoto). For these devices, the control is not created and 
keep_iface is simply set to true.

Signed-off-by: Andreas Böhler <dev@aboehler.at>

Comments

Takashi Sakamoto Aug. 31, 2018, 2:49 a.m. UTC | #1 
Hi,

On Aug 31 2018 04:57, news@aboehler.at wrote:
> I own a Phonic Helix Board 12 Universal USB sound mixer that I couldn't
> get to work properly on my box. After some debugging, it turned out that
> the playback stream is muted when the capture interface is brought down.
> 
> Since I was on 4.14, I implemented a quirk there and wanted to port it
> to 4.18 before submitting it - just to find out that the keep_iface
> control is very much the same approach.
> 
> This patch adds a mixer quirk for known devices such as the Phonic Helix
> Board 12 Universal and the Behringer FCA610 (tested and reported by
> Takashi Sakamoto). For these devices, the control is not created and
> keep_iface is simply set to true.
> 
> Signed-off-by: Andreas Böhler <dev@aboehler.at>

Some supplemental comment from me. This device is an application of
DM1500 ASIC designed by BridgeCo. AG (currently reorganized as ArchWave
AG). This ASIC has two interfaces; for USB 2.0 and IEEE 1394 bus.
(ALSA bebob driver is for the interface of IEEE 1394 bus.)

As long as we tested, the interface for USB 2.0 has a quirk for in/out
EPs to share sampling module on the ASIC, thus below scenario bring
unexpected results:

  - start packet streaming on an EP, then start/stop on opposite EP
  - start packet streaming on an EP at a certain sampling rate, then
    start on opposite EP at different sampling rate.

In my experience, both EPs touch sampling module without any any lock
primitives on the device, thus opening/closing the EPs bring state
change of the sampling module.

> diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
> index c63c84b54969..f11208fb4e58 100644
> --- a/sound/usb/mixer.c
> +++ b/sound/usb/mixer.c
> @@ -3474,9 +3474,15 @@ int snd_usb_create_mixer(struct snd_usb_audio *chip, int ctrlif,
>   	if (err < 0)
>   		goto _error;
>   
> -	err = create_keep_iface_ctl(mixer);
> -	if (err < 0)
> -		goto _error;
> +	/* For known cards, make the keep_iface quirk static
> +	 * otherwiese, add a mixer control */
> +	if(snd_usb_mixer_keep_iface_static_quirk(mixer)) {
> +		mixer->chip->keep_iface = true;
> +	} else {
> +		err = create_keep_iface_ctl(mixer);
> +		if (err < 0)
> +			goto _error;
> +	}
>   
>   	snd_usb_mixer_apply_create_quirk(mixer);
>   
> diff --git a/sound/usb/mixer_quirks.c b/sound/usb/mixer_quirks.c
> index cbfb48bdea51..414aa1f3a170 100644
> --- a/sound/usb/mixer_quirks.c
> +++ b/sound/usb/mixer_quirks.c
> @@ -2000,3 +2000,13 @@ void snd_usb_mixer_fu_apply_quirk(struct usb_mixer_interface *mixer,
>   	}
>   }
>   
> +bool snd_usb_mixer_keep_iface_static_quirk(struct usb_mixer_interface *mixer)
> +{
> +	switch (mixer->chip->usb_id) {
> +	case USB_ID(0x170b, 0x0015): /* Phonic Helix Board 12 Universal */
> +	case USB_ID(0x1397, 0x0003): /* Behringer FCA 610 */
> +		return true;
> +	}
> +
> +	return false;
> +}
> diff --git a/sound/usb/mixer_quirks.h b/sound/usb/mixer_quirks.h
> index 52be26db558f..5853e86adffd 100644
> --- a/sound/usb/mixer_quirks.h
> +++ b/sound/usb/mixer_quirks.h
> @@ -14,6 +14,8 @@ void snd_usb_mixer_fu_apply_quirk(struct usb_mixer_interface *mixer,
>   				  struct usb_mixer_elem_info *cval, int unitid,
>   				  struct snd_kcontrol *kctl);
>   
> +bool snd_usb_mixer_keep_iface_static_quirk(struct usb_mixer_interface *mixer);
> +
>   #ifdef CONFIG_PM
>   void snd_usb_mixer_resume_quirk(struct usb_mixer_interface *mixer);
>   #endif

As long as I tested, this is not enough solution. I can receive
unexpected result in a below case:

1. start playback substream at 48.0 kHz
2. hear expected sound from the device
3. start capture substream at 44.1kHz
4. hear no sound at a short time, then hear sound again but detuned

The snd-usb-audio module needs a quirk to have restriction about
sampling rate for both playback/capture substream with reference
counting.

Unfortunately, I have little time for this issue (I started my work
for the other issues at development cycle to v4.20 kernel), however
prepared for repository to backport your patch to v4.17 kernel or later:
https://github.com/takaswie/snd-usb-improve

You can see this take2 patch in 'topic/DM1500-quirk-take2' branch. I'm
happy if this helps your work.

For your information, there're some models based on the DM1500 ASIC. As
long as I know:
  - Phonic Helix Board 12/18/24
  - Phonic FireFly 202/302/808
  - Behringer FCA610/FCA1616
  - Behringer XENYX UFX1204/UFX1604

It's reasonable that this issue influences all of them.


Thanks

Takashi Sakamoto
diff mbox series

Patch

diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
index c63c84b54969..f11208fb4e58 100644
--- a/sound/usb/mixer.c
+++ b/sound/usb/mixer.c
@@ -3474,9 +3474,15 @@  int snd_usb_create_mixer(struct snd_usb_audio *chip, int ctrlif,
 	if (err < 0)
 		goto _error;
 
-	err = create_keep_iface_ctl(mixer);
-	if (err < 0)
-		goto _error;
+	/* For known cards, make the keep_iface quirk static
+	 * otherwiese, add a mixer control */
+	if(snd_usb_mixer_keep_iface_static_quirk(mixer)) {
+		mixer->chip->keep_iface = true;
+	} else {
+		err = create_keep_iface_ctl(mixer);
+		if (err < 0)
+			goto _error;
+	}
 
 	snd_usb_mixer_apply_create_quirk(mixer);
 
diff --git a/sound/usb/mixer_quirks.c b/sound/usb/mixer_quirks.c
index cbfb48bdea51..414aa1f3a170 100644
--- a/sound/usb/mixer_quirks.c
+++ b/sound/usb/mixer_quirks.c
@@ -2000,3 +2000,13 @@  void snd_usb_mixer_fu_apply_quirk(struct usb_mixer_interface *mixer,
 	}
 }
 
+bool snd_usb_mixer_keep_iface_static_quirk(struct usb_mixer_interface *mixer)
+{
+	switch (mixer->chip->usb_id) {
+	case USB_ID(0x170b, 0x0015): /* Phonic Helix Board 12 Universal */
+	case USB_ID(0x1397, 0x0003): /* Behringer FCA 610 */
+		return true;
+	}
+
+	return false;
+}
diff --git a/sound/usb/mixer_quirks.h b/sound/usb/mixer_quirks.h
index 52be26db558f..5853e86adffd 100644
--- a/sound/usb/mixer_quirks.h
+++ b/sound/usb/mixer_quirks.h
@@ -14,6 +14,8 @@  void snd_usb_mixer_fu_apply_quirk(struct usb_mixer_interface *mixer,
 				  struct usb_mixer_elem_info *cval, int unitid,
 				  struct snd_kcontrol *kctl);
 
+bool snd_usb_mixer_keep_iface_static_quirk(struct usb_mixer_interface *mixer);
+
 #ifdef CONFIG_PM
 void snd_usb_mixer_resume_quirk(struct usb_mixer_interface *mixer);
 #endif