[PATCH] ANDROID: sound: usb: Add vendor's hooking interface

[PATCH] ANDROID: sound: usb: Add vendor's hooking interface

[JaeHun Jung]

In mobile, a co-processor is used when using USB audio
to improve power consumption.
hooking is required for sync-up when operating
the co-processor. So register call-back function.
The main operation of the call-back function is as follows:
- Initialize the co-processor by transmitting data
  when initializing.
- Change the co-processor setting value through
  the interface function.
- Configure sampling rate
- pcm open/close

Bug: 156315379

Change-Id: I32e1dd408e64aaef68ee06c480c4b4d4c95546dc
Signed-off-by: JaeHun Jung <jh0801.jung@samsung.com>
---
 sound/usb/card.c     | 16 ++++++++++++++++
 sound/usb/card.h     |  1 +
 sound/usb/clock.c    |  5 +++++
 sound/usb/pcm.c      | 33 +++++++++++++++++++++++++++++++++
 sound/usb/usbaudio.h | 30 ++++++++++++++++++++++++++++++
 5 files changed, 85 insertions(+)

diff --git a/sound/usb/card.c b/sound/usb/card.c
index fd6fd17..2f3fa14 100644
--- a/sound/usb/card.c
+++ b/sound/usb/card.c
@@ -111,6 +111,7 @@ MODULE_PARM_DESC(skip_validation, "Skip unit descriptor validation (default: no)
 static DEFINE_MUTEX(register_mutex);
 static struct snd_usb_audio *usb_chip[SNDRV_CARDS];
 static struct usb_driver usb_audio_driver;
+struct snd_usb_audio_vendor_ops *usb_audio_ops;
 
 /*
  * disconnect streams
@@ -210,6 +211,12 @@ static int snd_usb_create_stream(struct snd_usb_audio *chip, int ctrlif, int int
 	return 0;
 }
 
+void snd_set_vender_interface(struct snd_usb_audio_vendor_ops *vendor_ops)
+{
+	usb_audio_ops = vendor_ops;
+}
+EXPORT_SYMBOL_GPL(snd_set_vender_interface);
+
 /*
  * parse audio control descriptor and create pcm/midi streams
  */
@@ -598,6 +605,9 @@ static int usb_audio_probe(struct usb_interface *intf,
 	if (err < 0)
 		return err;
 
+	if (usb_audio_ops && usb_audio_ops->vendor_conn)
+		usb_audio_ops->vendor_conn(intf, dev);
+
 	/*
 	 * found a config.  now register to ALSA
 	 */
@@ -653,6 +663,9 @@ static int usb_audio_probe(struct usb_interface *intf,
 	}
 	dev_set_drvdata(&dev->dev, chip);
 
+	if (usb_audio_ops && usb_audio_ops->vendor_usb_add_ctls)
+		usb_audio_ops->vendor_usb_add_ctls(chip, 0);
+
 	/*
 	 * For devices with more than one control interface, we assume the
 	 * first contains the audio controls. We might need a more specific
@@ -737,6 +750,9 @@ static void usb_audio_disconnect(struct usb_interface *intf)
 
 	card = chip->card;
 
+	if (usb_audio_ops && usb_audio_ops->vendor_disc)
+		usb_audio_ops->vendor_disc();
+
 	mutex_lock(&register_mutex);
 	if (atomic_inc_return(&chip->shutdown) == 1) {
 		struct snd_usb_stream *as;
diff --git a/sound/usb/card.h b/sound/usb/card.h
index 395403a..a55bb4c 100644
--- a/sound/usb/card.h
+++ b/sound/usb/card.h
@@ -175,5 +175,6 @@ struct snd_usb_stream {
 	struct snd_usb_substream substream[2];
 	struct list_head list;
 };
+void snd_set_vender_interface(struct snd_usb_audio_vendor_ops *vendor_ops);
 
 #endif /* __USBAUDIO_CARD_H */
diff --git a/sound/usb/clock.c b/sound/usb/clock.c
index b118cf9..0ceeccb 100644
--- a/sound/usb/clock.c
+++ b/sound/usb/clock.c
@@ -642,8 +642,13 @@ static int set_sample_rate_v2v3(struct snd_usb_audio *chip, int iface,
 	 * interface is active. */
 	if (rate != prev_rate) {
 		usb_set_interface(dev, iface, 0);
+		if (usb_audio_ops && usb_audio_ops->vendor_set_intf)
+			usb_audio_ops->vendor_set_intf(dev, alts, iface, 0);
 		snd_usb_set_interface_quirk(dev);
 		usb_set_interface(dev, iface, fmt->altsetting);
+		if (usb_audio_ops && usb_audio_ops->vendor_set_intf)
+			usb_audio_ops->vendor_set_intf(dev, alts, iface,
+					fmt->altsetting);
 		snd_usb_set_interface_quirk(dev);
 	}
 
diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
index a4e4064..6cdacac 100644
--- a/sound/usb/pcm.c
+++ b/sound/usb/pcm.c
@@ -134,6 +134,9 @@ static struct audioformat *find_format(struct snd_usb_substream *subs)
 			found = fp;
 			cur_attr = attr;
 		}
+
+		if (usb_audio_ops && usb_audio_ops->vendor_pcm_binterval)
+			usb_audio_ops->vendor_pcm_binterval(fp, found, &cur_attr, &attr);
 	}
 	return found;
 }
@@ -568,6 +571,9 @@ static int set_format(struct snd_usb_substream *subs, struct audioformat *fmt)
 		}
 		dev_dbg(&dev->dev, "setting usb interface %d:%d\n",
 			fmt->iface, fmt->altsetting);
+		if (usb_audio_ops && usb_audio_ops->vendor_set_pcm_intf)
+			usb_audio_ops->vendor_set_pcm_intf(dev, fmt->iface,
+					fmt->altsetting, subs->direction);
 		snd_usb_set_interface_quirk(dev);
 	}
 
@@ -891,6 +897,15 @@ static int snd_usb_pcm_prepare(struct snd_pcm_substream *substream)
 	struct usb_interface *iface;
 	int ret;
 
+	if (usb_audio_ops && usb_audio_ops->vendor_set_pcmbuf) {
+		ret = usb_audio_ops->vendor_set_pcmbuf(subs->dev);
+
+		if (ret < 0) {
+			dev_err(&subs->dev->dev, "pcm buf transfer failed\n");
+			return ret;
+		}
+	}
+
 	if (! subs->cur_audiofmt) {
 		dev_err(&subs->dev->dev, "no format is specified!\n");
 		return -ENXIO;
@@ -924,6 +939,15 @@ static int snd_usb_pcm_prepare(struct snd_pcm_substream *substream)
 		if (ret < 0)
 			goto unlock;
 
+		if (usb_audio_ops && usb_audio_ops->vendor_set_rate) {
+			subs->need_setup_ep = false;
+			usb_audio_ops->vendor_set_rate(
+					subs->cur_audiofmt->iface,
+					subs->cur_rate,
+					subs->cur_audiofmt->altsetting);
+			goto unlock;
+		}
+
 		ret = configure_endpoint(subs);
 		if (ret < 0)
 			goto unlock;
@@ -1333,6 +1357,9 @@ static int snd_usb_pcm_open(struct snd_pcm_substream *substream)
 	struct snd_usb_substream *subs = &as->substream[direction];
 	int ret;
 
+	if (usb_audio_ops && usb_audio_ops->vendor_pcm_con)
+		usb_audio_ops->vendor_pcm_con(true, direction);
+
 	subs->interface = -1;
 	subs->altset_idx = 0;
 	runtime->hw = snd_usb_hardware;
@@ -1361,12 +1388,18 @@ static int snd_usb_pcm_close(struct snd_pcm_substream *substream)
 	struct snd_usb_substream *subs = &as->substream[direction];
 	int ret;
 
+	if (usb_audio_ops && usb_audio_ops->vendor_pcm_con)
+		usb_audio_ops->vendor_pcm_con(false, direction);
+
 	snd_media_stop_pipeline(subs);
 
 	if (!as->chip->keep_iface &&
 	    subs->interface >= 0 &&
 	    !snd_usb_lock_shutdown(subs->stream->chip)) {
 		usb_set_interface(subs->dev, subs->interface, 0);
+		if (usb_audio_ops && usb_audio_ops->vendor_set_pcm_intf)
+			usb_audio_ops->vendor_set_pcm_intf(subs->dev,
+					subs->interface, 0, direction);
 		subs->interface = -1;
 		ret = snd_usb_pcm_change_state(subs, UAC3_PD_STATE_D1);
 		snd_usb_unlock_shutdown(subs->stream->chip);
diff --git a/sound/usb/usbaudio.h b/sound/usb/usbaudio.h
index 1c892c7..a2fd8a4 100644
--- a/sound/usb/usbaudio.h
+++ b/sound/usb/usbaudio.h
@@ -124,4 +124,34 @@ void snd_usb_unlock_shutdown(struct snd_usb_audio *chip);
 extern bool snd_usb_use_vmalloc;
 extern bool snd_usb_skip_validation;
 
+/* for vender function mapping */
+extern struct snd_usb_audio_vendor_ops *usb_audio_ops;
+
+/* USB audio interface function for audio core */
+struct snd_usb_audio_vendor_ops {
+	/* Set descriptors and memory map */
+	void (*vendor_conn)(struct usb_interface *intf,
+			struct usb_device *udev);
+	/* Set disconnection */
+	void (*vendor_disc)(void);
+	/* Set interface info and setting value */
+	int (*vendor_set_intf)(struct usb_device *udev,
+			struct usb_host_interface *alts, int iface, int alt);
+	/* Set sample rate */
+	int (*vendor_set_rate)(int iface, int rate, int alt);
+	/* Alloc pcm buffer */
+	int (*vendor_set_pcmbuf)(struct usb_device *udev);
+	/* Set pcm interface */
+	int (*vendor_set_pcm_intf)(struct usb_device *udev,
+			int iface, int alt, int direction);
+	/* informed whether pcm open/close to vendor */
+	void (*vendor_pcm_con)(int onoff, int direction);
+	/* set datainterval */
+	void (*vendor_pcm_binterval)(void *fp, void *found,
+			int *cur_attr, int *attr);
+	/* control USB F/W */
+	int (*vendor_usb_add_ctls)(struct snd_usb_audio *chip,
+				unsigned long private_value);
+};
+
 #endif /* __USBAUDIO_H */
-- 
2.7.4

Re: [PATCH] ANDROID: sound: usb: Add vendor's hooking interface

[Takashi Sakamoto]

Hi,

On Wed, Jun 17, 2020 at 11:18:24AM +0900, JaeHun Jung wrote:
> In mobile, a co-processor is used when using USB audio
> to improve power consumption.
> hooking is required for sync-up when operating
> the co-processor. So register call-back function.
> The main operation of the call-back function is as follows:
> - Initialize the co-processor by transmitting data
>   when initializing.
> - Change the co-processor setting value through
>   the interface function.
> - Configure sampling rate
> - pcm open/close
> 
> Bug: 156315379
> 
> Change-Id: I32e1dd408e64aaef68ee06c480c4b4d4c95546dc
> Signed-off-by: JaeHun Jung <jh0801.jung@samsung.com>
> ---
>  sound/usb/card.c     | 16 ++++++++++++++++
>  sound/usb/card.h     |  1 +
>  sound/usb/clock.c    |  5 +++++
>  sound/usb/pcm.c      | 33 +++++++++++++++++++++++++++++++++
>  sound/usb/usbaudio.h | 30 ++++++++++++++++++++++++++++++
>  5 files changed, 85 insertions(+)

> diff --git a/sound/usb/card.c b/sound/usb/card.c
> index fd6fd17..2f3fa14 100644
> --- a/sound/usb/card.c
> +++ b/sound/usb/card.c
> @@ -111,6 +111,7 @@ MODULE_PARM_DESC(skip_validation, "Skip unit descriptor validation (default: no)
>  static DEFINE_MUTEX(register_mutex);
>  static struct snd_usb_audio *usb_chip[SNDRV_CARDS];
>  static struct usb_driver usb_audio_driver;
> +struct snd_usb_audio_vendor_ops *usb_audio_ops;
>  
>  /*
>   * disconnect streams
> @@ -210,6 +211,12 @@ static int snd_usb_create_stream(struct snd_usb_audio *chip, int ctrlif, int int
>  	return 0;
>  }
>  
> +void snd_set_vender_interface(struct snd_usb_audio_vendor_ops *vendor_ops)
> +{
> +	usb_audio_ops = vendor_ops;
> +}
> +EXPORT_SYMBOL_GPL(snd_set_vender_interface);

I think the symbol name has typo; 'vender' against 'vendor'.

Anyway, this feature is not widely used at present. I suggest to add
kernel configuration for the feature at kernel compilation time, IMO.


Regards

Takashi Sakamoto

Re: [PATCH] ANDROID: sound: usb: Add vendor's hooking interface

[Christoph Hellwig]

On Wed, Jun 17, 2020 at 11:18:24AM +0900, JaeHun Jung wrote:
> In mobile, a co-processor is used when using USB audio
> to improve power consumption.
> hooking is required for sync-up when operating
> the co-processor. So register call-back function.
> The main operation of the call-back function is as follows:
> - Initialize the co-processor by transmitting data
>   when initializing.
> - Change the co-processor setting value through
>   the interface function.
> - Configure sampling rate
> - pcm open/close
> 
> Bug: 156315379

None of that is acceptable without an in-tree user.  Never mind that
the interface just seems horrible.  I'm pretty sure we could do
somehting much better and more specific based on the actual needs of
the user, so please post it.

Re: [PATCH] ANDROID: sound: usb: Add vendor's hooking interface

[Pierre-Louis Bossart]

On 6/16/20 9:18 PM, JaeHun Jung wrote:
> In mobile, a co-processor is used when using USB audio
> to improve power consumption.
> hooking is required for sync-up when operating
> the co-processor. So register call-back function.
> The main operation of the call-back function is as follows:
> - Initialize the co-processor by transmitting data
>    when initializing.
> - Change the co-processor setting value through
>    the interface function.
> - Configure sampling rate
> - pcm open/close

Thank you for this patch. With the removal of the 3.5mm jack on a number 
of platforms, and improvements to reduce power consumption on a variety 
of hosts there's indeed a need to enhance sound/usb in the kernel.

> 
> Bug: 156315379
> 
> Change-Id: I32e1dd408e64aaef68ee06c480c4b4d4c95546dc
> Signed-off-by: JaeHun Jung <jh0801.jung@samsung.com>

You probably want to remove bug references and Change-Id if they are not 
publicly visible

> @@ -891,6 +897,15 @@ static int snd_usb_pcm_prepare(struct snd_pcm_substream *substream)
>   	struct usb_interface *iface;
>   	int ret;
>   
> +	if (usb_audio_ops && usb_audio_ops->vendor_set_pcmbuf) {
> +		ret = usb_audio_ops->vendor_set_pcmbuf(subs->dev);
> +
> +		if (ret < 0) {
> +			dev_err(&subs->dev->dev, "pcm buf transfer failed\n");
> +			return ret;
> +		}
> +	}
> +
>   	if (! subs->cur_audiofmt) {
>   		dev_err(&subs->dev->dev, "no format is specified!\n");
>   		return -ENXIO;
> @@ -924,6 +939,15 @@ static int snd_usb_pcm_prepare(struct snd_pcm_substream *substream)
>   		if (ret < 0)
>   			goto unlock;
>   
> +		if (usb_audio_ops && usb_audio_ops->vendor_set_rate) {
> +			subs->need_setup_ep = false;
> +			usb_audio_ops->vendor_set_rate(
> +					subs->cur_audiofmt->iface,
> +					subs->cur_rate,
> +					subs->cur_audiofmt->altsetting);
> +			goto unlock;
> +		}
> +
>   		ret = configure_endpoint(subs);
>   		if (ret < 0)
>   			goto unlock;
> @@ -1333,6 +1357,9 @@ static int snd_usb_pcm_open(struct snd_pcm_substream *substream)
>   	struct snd_usb_substream *subs = &as->substream[direction];
>   	int ret;
>   
> +	if (usb_audio_ops && usb_audio_ops->vendor_pcm_con)
> +		usb_audio_ops->vendor_pcm_con(true, direction);
> +
>   	subs->interface = -1;
>   	subs->altset_idx = 0;
>   	runtime->hw = snd_usb_hardware;
> @@ -1361,12 +1388,18 @@ static int snd_usb_pcm_close(struct snd_pcm_substream *substream)
>   	struct snd_usb_substream *subs = &as->substream[direction];
>   	int ret;
>   
> +	if (usb_audio_ops && usb_audio_ops->vendor_pcm_con)
> +		usb_audio_ops->vendor_pcm_con(false, direction);
> +

can you elaborate on the PCM streaming parts? if you allocate a buffer 
for the co-processor to deal with PCM data, will you end-up copying data 
from the existing ring buffers allocated with vmalloc() into your 
coprocessor?

I was instead thinking of having a different 'mode' where the PCM 
buffers are not allocated by sound/usb, but instead allocated by the 
driver taking care of the low-power USB path, so that apps can directly 
write into ring buffers that are handled by the coprocessor/offload unit.

Thanks
-Pierre