On 1/25/23 21:14, Wesley Cheng wrote: > Several Qualcomm SoCs have a dedicated audio DSP, which has the ability to > support USB sound devices. This vendor driver will implement the required > handshaking with the DSP, in order to pass along required resources that > will be utilized by the DSP's USB SW. The communication channel used for > this handshaking will be using the QMI protocol. Required resources > include: > - Allocated secondary event ring address > - EP transfer ring address > - Interrupter number > > The above information will allow for the audio DSP to execute USB transfers > over the USB bus. It will also be able to support devices that have an > implicit feedback and sync endpoint as well. Offloading these data > transfers will allow the main/applications processor to enter lower CPU > power modes, and sustain a longer duration in those modes. > > Audio offloading is initiated with the following sequence: > 1. Userspace configures to route audio playback to USB backend and starts > playback on the platform soundcard. what happens if the DSP driver is probed after the USB one? Or vice-versa? Userspace needs to be notified of what is detected at the kernel level, I don't see how we can assume a specific route is always present. > +config QC_USB_AUDIO_OFFLOAD > + tristate "Qualcomm Audio Offload driver" USB Audio Offload > +struct uaudio_dev { > + struct usb_device *udev; > + /* audio control interface */ > + struct usb_host_interface *ctrl_intf; > + unsigned int card_num; > + unsigned int usb_core_id; > + atomic_t in_use; > + struct kref kref; > + wait_queue_head_t disconnect_wq; > + > + /* interface specific */ > + int num_intf; > + struct intf_info *info; > + struct snd_usb_audio *chip; > +}; > + > +static struct uaudio_dev uadev[SNDRV_CARDS]; I don't follow what this array is? Does this shadow all possible cards, even non-USB ones? > +static struct uaudio_qmi_dev *uaudio_qdev; > +static struct uaudio_qmi_svc *uaudio_svc; > +static DEFINE_MUTEX(qdev_mutex); > +/** > + * disable_audio_stream() - disable usb snd endpoints > + * @subs: usb substream > + * > + * Closes the USB SND endpoints associated with the current audio stream > + * used. This will decrement the USB SND endpoint opened reference count. > + * > + */ > +static void disable_audio_stream(struct snd_usb_substream *subs) > +{ > + struct snd_usb_audio *chip = subs->stream->chip; > + > + if (subs->data_endpoint || subs->sync_endpoint) { > + close_endpoints(chip, subs); > + > + mutex_lock(&chip->mutex); > + subs->cur_audiofmt = NULL; > + mutex_unlock(&chip->mutex); can you explain why the format selection is protected by a mutex? I don't quite get what level of concurrency might happen here? > + } > + > + snd_usb_autosuspend(chip); > +} > + > +/** > + * enable_audio_stream() - enable usb snd endpoints > + * @subs: usb substream > + * @pcm_format: pcm format requested > + * @channels: number of channels > + * @cur_rate: sample rate > + * @datainterval: interval > + * > + * Opens all USB SND endpoints used for the data interface. This will increment > + * the USB SND endpoint's opened count. Requests to keep the interface resumed > + * until the audio stream is stopped. Will issue the USB set interface control > + * message to enable the data interface. > + * > + */ > +static int enable_audio_stream(struct snd_usb_substream *subs, > + snd_pcm_format_t pcm_format, > + unsigned int channels, unsigned int cur_rate, > + int datainterval) > +{ > + struct snd_usb_audio *chip = subs->stream->chip; > + struct snd_pcm_hw_params params; > + const struct audioformat *fmt; > + int ret; > + > + _snd_pcm_hw_params_any(¶ms); > + _snd_pcm_hw_param_set(¶ms, SNDRV_PCM_HW_PARAM_FORMAT, > + (__force int) pcm_format, 0); > + _snd_pcm_hw_param_set(¶ms, SNDRV_PCM_HW_PARAM_CHANNELS, > + channels, 0); > + _snd_pcm_hw_param_set(¶ms, SNDRV_PCM_HW_PARAM_RATE, > + cur_rate, 0); > + > + pm_runtime_barrier(&chip->intf[0]->dev); > + snd_usb_autoresume(chip); > + > + fmt = find_format(&subs->fmt_list, pcm_format, cur_rate, > + channels, datainterval, subs); > + if (!fmt) { > + dev_err(uaudio_qdev->dev, > + "cannot find format: format = %#x, rate = %d, ch = %d\n", > + pcm_format, cur_rate, channels); > + return -EINVAL; > + } > + > + if (atomic_read(&chip->shutdown)) { > + dev_err(uaudio_qdev->dev, "chip already shutdown\n"); > + ret = -ENODEV; > + } else { > + if (subs->data_endpoint) > + close_endpoints(chip, subs); > + > + subs->data_endpoint = snd_usb_endpoint_open(chip, fmt, > + ¶ms, false); > + if (!subs->data_endpoint) { > + dev_err(uaudio_qdev->dev, "failed to open data endpoint\n"); > + return -EINVAL; > + } > + > + if (fmt->sync_ep) { > + subs->sync_endpoint = snd_usb_endpoint_open(chip, > + fmt, ¶ms, true); > + if (!subs->sync_endpoint) { > + dev_err(uaudio_qdev->dev, > + "failed to open sync endpoint\n"); > + return -EINVAL; > + } > + > + subs->data_endpoint->sync_source = subs->sync_endpoint; > + } > + > + mutex_lock(&chip->mutex); > + subs->cur_audiofmt = fmt; > + mutex_unlock(&chip->mutex); > + > + if (subs->sync_endpoint) { > + ret = snd_usb_endpoint_prepare(chip, subs->sync_endpoint); > + if (ret < 0) > + return ret; > + } > + > + ret = snd_usb_endpoint_prepare(chip, subs->data_endpoint); > + if (ret < 0) > + return ret; what happens in those two error cases? Should the format selected above remain set even though the prepare failed? > + > + dev_dbg(uaudio_qdev->dev, > + "selected %s iface:%d altsetting:%d datainterval:%dus\n", > + subs->direction ? "capture" : "playback", > + fmt->iface, fmt->altsetting, > + (1 << fmt->datainterval) * > + (subs->dev->speed >= USB_SPEED_HIGH ? > + BUS_INTERVAL_HIGHSPEED_AND_ABOVE : > + BUS_INTERVAL_FULL_SPEED)); > + } > + > + return 0; > +} <snip> > diff --git a/sound/usb/qcom/usb_audio_qmi_v01.c b/sound/usb/qcom/usb_audio_qmi_v01.c > new file mode 100644 > index 000000000000..95ae434f0a41 > --- /dev/null > +++ b/sound/usb/qcom/usb_audio_qmi_v01.c > @@ -0,0 +1,892 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (c) 2022 Qualcomm Innovation Center, Inc. All rights reserved. > + */ > + > +#include <linux/soc/qcom/qmi.h> > + > +#include "usb_audio_qmi_v01.h" > + > +static struct qmi_elem_info mem_info_v01_ei[] = { > + { > + .data_type = QMI_UNSIGNED_8_BYTE, > + .elem_len = 1, > + .elem_size = sizeof(u64), > + .array_type = NO_ARRAY, > + .tlv_type = 0, > + .offset = offsetof(struct mem_info_v01, va), > + }, > + { > + .data_type = QMI_UNSIGNED_8_BYTE, > + .elem_len = 1, > + .elem_size = sizeof(u64), > + .array_type = NO_ARRAY, > + .tlv_type = 0, > + .offset = offsetof(struct mem_info_v01, pa), > + }, > + { > + .data_type = QMI_UNSIGNED_4_BYTE, > + .elem_len = 1, > + .elem_size = sizeof(u32), > + .array_type = NO_ARRAY, > + .tlv_type = 0, > + .offset = offsetof(struct mem_info_v01, size), > + }, > + { > + .data_type = QMI_EOTI, > + .array_type = NO_ARRAY, > + .tlv_type = QMI_COMMON_TLV_TYPE, > + }, <snip> > + { > + .data_type = QMI_EOTI, > + .array_type = NO_ARRAY, > + .tlv_type = QMI_COMMON_TLV_TYPE, > + }, > +}; Are those dozens of descriptors needed? They look mostly the same, not sure how anyone could review this.