lkml.org 
[lkml]   [2023]   [Mar]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH v3 15/28] sound: usb: Introduce QC USB SND offloading support
    From

    > +config QC_USB_AUDIO_OFFLOAD
    > + tristate "Qualcomm Audio Offload driver"

    "Qualcomm USB Audio Offload driver"

    > + select SND_PCM

    depends on SND_USB_AUDIO ?

    > + help
    > + Say Y here to enable the Qualcomm USB audio offloading feature
    > +
    > + This module sets up the required QMI stream enable/disable
    > + responses to requests generated by the audio DSP. It passes the
    > + USB transfer resource references, so that the audio DSP can issue
    > + USB transfers to the host controller.
    > +
    > + To compile this driver as a module, choose M here: the module
    > + will be called qc-audio-offload.

    > +#include <linux/ctype.h>
    > +#include <linux/moduleparam.h>
    > +#include <linux/module.h>
    > +#include <linux/usb.h>
    > +#include <linux/init.h>

    alphabetical order?

    > +#include <linux/usb/hcd.h>
    > +#include <linux/usb/xhci-intr.h>
    > +#include <linux/usb/quirks.h>
    > +#include <linux/usb/audio.h>
    > +#include <linux/usb/audio-v2.h>
    > +#include <linux/usb/audio-v3.h>
    > +#include <linux/soc/qcom/qmi.h>
    > +#include <linux/iommu.h>
    > +#include <linux/dma-mapping.h>
    > +#include <linux/dma-map-ops.h>
    > +#include <sound/q6usboffload.h>
    > +
    > +#include <sound/control.h>
    > +#include <sound/core.h>
    > +#include <sound/info.h>
    > +#include <sound/pcm.h>
    > +#include <sound/pcm_params.h>
    > +#include <sound/initval.h>
    > +
    > +#include <sound/soc.h>
    > +#include <sound/soc-usb.h>
    > +#include "../usbaudio.h"
    > +#include "../card.h"
    > +#include "../midi.h"
    > +#include "../mixer.h"
    > +#include "../proc.h"
    > +#include "../quirks.h"
    > +#include "../endpoint.h"
    > +#include "../helper.h"
    > +#include "../pcm.h"
    > +#include "../format.h"
    > +#include "../power.h"
    > +#include "../stream.h"
    > +#include "../media.h"

    this looks weird, are all these needed?

    > +
    > +#define SND_PCM_CARD_NUM_MASK 0xffff0000
    > +#define SND_PCM_DEV_NUM_MASK 0xff00
    > +#define SND_PCM_STREAM_DIRECTION 0xff

    The prefix looks wrong, or it duplicates SND_PCM stuff?

    > +
    > +/* iommu resource parameters and management */
    > +#define PREPEND_SID_TO_IOVA(iova, sid) ((u64)(((u64)(iova)) | \
    > + (((u64)sid) << 32)))
    > +#define IOVA_BASE 0x1000
    > +#define IOVA_XFER_RING_BASE (IOVA_BASE + PAGE_SIZE * (SNDRV_CARDS + 1))
    > +#define IOVA_XFER_BUF_BASE (IOVA_XFER_RING_BASE + PAGE_SIZE * SNDRV_CARDS * 32)
    > +#define IOVA_XFER_RING_MAX (IOVA_XFER_BUF_BASE - PAGE_SIZE)
    > +#define IOVA_XFER_BUF_MAX (0xfffff000 - PAGE_SIZE)
    > +
    > +#define MAX_XFER_BUFF_LEN (24 * PAGE_SIZE)
    > +
    > +struct iova_info {
    > + struct list_head list;
    > + unsigned long start_iova;
    > + size_t size;
    > + bool in_use;
    > +};
    > +
    > +struct intf_info {
    > + unsigned long data_xfer_ring_va;
    > + size_t data_xfer_ring_size;
    > + unsigned long sync_xfer_ring_va;
    > + size_t sync_xfer_ring_size;
    > + unsigned long xfer_buf_va;
    > + size_t xfer_buf_size;
    > + phys_addr_t xfer_buf_pa;
    > + unsigned int data_ep_pipe;
    > + unsigned int sync_ep_pipe;
    > + u8 *xfer_buf;
    > + u8 intf_num;
    > + u8 pcm_card_num;
    > + u8 pcm_dev_num;
    > + u8 direction;
    > + bool in_use;
    > +};
    > +
    > +struct uaudio_qmi_dev {
    > + struct device *dev;
    > + u32 sid;
    > + u32 intr_num;
    > + struct xhci_interrupter *ir;
    > + struct xhci_ring *sec_ring;
    > + struct iommu_domain *domain;
    > +
    > + /* list to keep track of available iova */
    > + struct list_head xfer_ring_list;
    > + size_t xfer_ring_iova_size;
    > + unsigned long curr_xfer_ring_iova;
    > + struct list_head xfer_buf_list;
    > + size_t xfer_buf_iova_size;
    > + unsigned long curr_xfer_buf_iova;
    > +
    > + /* bit fields representing pcm card enabled */
    > + unsigned long card_slot;
    > + /* indicate event ring mapped or not */
    > + bool er_mapped;
    > + /* reference count to number of possible consumers */
    > + atomic_t qdev_in_use;
    > + /* idx to last udev card number plugged in */
    > + unsigned int last_card_num;
    > +};
    > +
    > +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;
    > +};

    This looks too detailed, isnt' that a duplication of other structures
    that exist in the regular USB audio part?

    There's nothing that looks QCOM-specific anyways.

    > +
    > +static struct uaudio_dev uadev[SNDRV_CARDS];
    > +static struct uaudio_qmi_dev *uaudio_qdev;
    > +static struct uaudio_qmi_svc *uaudio_svc;
    > +static DEFINE_MUTEX(qdev_mutex);
    > +
    > +struct uaudio_qmi_svc {
    > + struct qmi_handle *uaudio_svc_hdl;
    > + struct work_struct qmi_disconnect_work;
    > + struct workqueue_struct *uaudio_wq;
    > + struct sockaddr_qrtr client_sq;
    > + bool client_connected;
    > +};
    > +
    > +enum mem_type {
    > + MEM_EVENT_RING,
    > + MEM_XFER_RING,
    > + MEM_XFER_BUF,
    > +};

    prefixes better than 'MEM'?

    > +
    > +/* Supported audio formats */
    > +enum usb_qmi_audio_format {
    > + USB_QMI_PCM_FORMAT_S8 = 0,
    > + USB_QMI_PCM_FORMAT_U8,
    > + USB_QMI_PCM_FORMAT_S16_LE,
    > + USB_QMI_PCM_FORMAT_S16_BE,
    > + USB_QMI_PCM_FORMAT_U16_LE,
    > + USB_QMI_PCM_FORMAT_U16_BE,
    > + USB_QMI_PCM_FORMAT_S24_LE,
    > + USB_QMI_PCM_FORMAT_S24_BE,
    > + USB_QMI_PCM_FORMAT_U24_LE,
    > + USB_QMI_PCM_FORMAT_U24_BE,
    > + USB_QMI_PCM_FORMAT_S24_3LE,
    > + USB_QMI_PCM_FORMAT_S24_3BE,
    > + USB_QMI_PCM_FORMAT_U24_3LE,
    > + USB_QMI_PCM_FORMAT_U24_3BE,
    > + USB_QMI_PCM_FORMAT_S32_LE,
    > + USB_QMI_PCM_FORMAT_S32_BE,
    > + USB_QMI_PCM_FORMAT_U32_LE,
    > + USB_QMI_PCM_FORMAT_U32_BE,
    > +};
    > +
    > +static enum usb_audio_device_speed_enum_v01
    > +get_speed_info(enum usb_device_speed udev_speed)
    > +{
    > + switch (udev_speed) {
    > + case USB_SPEED_LOW:
    > + return USB_AUDIO_DEVICE_SPEED_LOW_V01;
    > + case USB_SPEED_FULL:
    > + return USB_AUDIO_DEVICE_SPEED_FULL_V01;
    > + case USB_SPEED_HIGH:
    > + return USB_AUDIO_DEVICE_SPEED_HIGH_V01;
    > + case USB_SPEED_SUPER:
    > + return USB_AUDIO_DEVICE_SPEED_SUPER_V01;
    > + case USB_SPEED_SUPER_PLUS:
    > + return USB_AUDIO_DEVICE_SPEED_SUPER_PLUS_V01;
    > + default:
    > + return USB_AUDIO_DEVICE_SPEED_INVALID_V01;
    > + }
    > +}

    this doesn't look qcom-specific, why is this here?

    > +
    > +static struct snd_usb_substream *find_substream(unsigned int card_num,
    > + unsigned int pcm_idx, unsigned int direction)
    > +{
    > + struct snd_usb_stream *as;
    > + struct snd_usb_substream *subs = NULL;
    > + struct snd_usb_audio *chip;
    > +
    > + chip = uadev[card_num].chip;
    > + if (!chip || atomic_read(&chip->shutdown))
    > + goto done;
    > +
    > + if (pcm_idx >= chip->pcm_devs)
    > + goto done;
    > +
    > + if (direction > SNDRV_PCM_STREAM_CAPTURE)
    > + goto done;
    > +
    > + list_for_each_entry(as, &chip->pcm_list, list) {
    > + if (as->pcm_index == pcm_idx) {
    > + subs = &as->substream[direction];
    > + goto done;
    > + }
    > + }
    > +
    > +done:
    > + return subs;
    > +}
    > +
    > +static int info_idx_from_ifnum(int card_num, int intf_num, bool enable)
    > +{
    > + int i;
    > +
    > + /*
    > + * default index 0 is used when info is allocated upon
    > + * first enable audio stream req for a pcm device
    > + */
    > + if (enable && !uadev[card_num].info)
    > + return 0;
    > +
    > + for (i = 0; i < uadev[card_num].num_intf; i++) {
    > + if (enable && !uadev[card_num].info[i].in_use)
    > + return i;
    > + else if (!enable &&
    > + uadev[card_num].info[i].intf_num == intf_num)
    > + return i;
    > + }
    > +
    > + return -EINVAL;
    > +}
    > +
    > +static int get_data_interval_from_si(struct snd_usb_substream *subs,
    > + u32 service_interval)
    > +{
    > + unsigned int bus_intval, bus_intval_mult, binterval;
    > +
    > + if (subs->dev->speed >= USB_SPEED_HIGH)
    > + bus_intval = BUS_INTERVAL_HIGHSPEED_AND_ABOVE;
    > + else
    > + bus_intval = BUS_INTERVAL_FULL_SPEED;
    > +
    > + if (service_interval % bus_intval)
    > + return -EINVAL;
    > +
    > + bus_intval_mult = service_interval / bus_intval;
    > + binterval = ffs(bus_intval_mult);
    > + if (!binterval || binterval > MAX_BINTERVAL_ISOC_EP)
    > + return -EINVAL;
    > +
    > + /* check if another bit is set then bail out */
    > + bus_intval_mult = bus_intval_mult >> binterval;
    > + if (bus_intval_mult)
    > + return -EINVAL;
    > +
    > + return (binterval - 1);
    > +}

    this doesn't look qcom-specific either, is this a set of helpers that
    need to be moved somewhere else?


    > +/**
    > + * 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);
    > + }
    > +
    > + snd_usb_autosuspend(chip);
    > +}

    also not qcom-specific?

    > +/* kref release callback when all streams are disabled */

    what does "all streams disabled" mean, and when does this happen?

    > +static void uaudio_dev_release(struct kref *kref)
    > +{
    > + struct uaudio_dev *dev = container_of(kref, struct uaudio_dev, kref);
    > +
    > + uaudio_event_ring_cleanup_free(dev);
    > + atomic_set(&dev->in_use, 0);
    > + wake_up(&dev->disconnect_wq);
    > +}


    > +static int __init qc_usb_audio_offload_init(void)
    > +{
    > + struct uaudio_qmi_svc *svc;
    > + int ret;
    > +
    > + ret = snd_usb_register_platform_ops(&offload_ops);
    > + if (ret < 0)
    > + return ret;

    so what happens if snd_usb is blacklisted or takes time to probe?

    This seems to assume a timing dependency when USB is initialized/probed
    first?

    > +
    > + svc = kzalloc(sizeof(struct uaudio_qmi_svc), GFP_KERNEL);
    > + if (!svc) {
    > + ret = -ENOMEM;
    > + goto unreg_ops;
    > + }
    > +
    > + svc->uaudio_wq = create_singlethread_workqueue("uaudio_svc");
    > + if (!svc->uaudio_wq) {
    > + ret = -ENOMEM;
    > + goto free_svc;
    > + }
    > +
    > + svc->uaudio_svc_hdl = kzalloc(sizeof(struct qmi_handle), GFP_KERNEL);
    > + if (!svc->uaudio_svc_hdl) {
    > + ret = -ENOMEM;
    > + goto free_wq;
    > + }
    > +
    > + ret = qmi_handle_init(svc->uaudio_svc_hdl,
    > + QMI_UAUDIO_STREAM_REQ_MSG_V01_MAX_MSG_LEN,
    > + &uaudio_svc_ops_options,
    > + &uaudio_stream_req_handlers);
    > + ret = qmi_add_server(svc->uaudio_svc_hdl, UAUDIO_STREAM_SERVICE_ID_V01,
    > + UAUDIO_STREAM_SERVICE_VERS_V01, 0);
    > +
    > + INIT_WORK(&svc->qmi_disconnect_work, qmi_disconnect_work);
    > + uaudio_svc = svc;
    > +
    > + return 0;
    > +
    > +free_wq:
    > + destroy_workqueue(svc->uaudio_wq);
    > +free_svc:
    > + kfree(svc);
    > +unreg_ops:
    > + snd_usb_unregister_platform_ops();
    > +
    > + return ret;
    > +}
    > +
    > +static void __exit qc_usb_audio_offload_exit(void)
    > +{
    > + struct uaudio_qmi_svc *svc = uaudio_svc;
    > +
    > + qmi_handle_release(svc->uaudio_svc_hdl);
    > + flush_workqueue(svc->uaudio_wq);
    > + destroy_workqueue(svc->uaudio_wq);
    > + kfree(svc);
    > + uaudio_svc = NULL;
    > + snd_usb_unregister_platform_ops();
    > +}
    > +
    > +module_init(qc_usb_audio_offload_init);
    > +module_exit(qc_usb_audio_offload_exit);

    what causes this module to be loaded?

    > +
    > +MODULE_DESCRIPTION("QC USB Audio Offloading");
    > +MODULE_LICENSE("GPL");
    > 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..1a0f2f579786
    > --- /dev/null
    > +++ b/sound/usb/qcom/usb_audio_qmi_v01.c
    > @@ -0,0 +1,892 @@
    > +// SPDX-License-Identifier: GPL-2.0
    > +/*
    > + * Copyright (c) 2022-2023 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),
    > + },

    maybe move these arrays to a different patch that we don't need to
    scroll-through or review...

    > + {
    > + .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,
    > + },
    > +};

    \
     
     \ /
      Last update: 2023-03-27 00:52    [W:4.083 / U:0.020 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site