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

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