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.j...@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