| Message ID | 20230308235751.495-1-quic_wcheng@quicinc.com |
|---|---|
| Headers | show |
| Series | Introduce QC USB SND audio offloading support | expand |
On Wed, Mar 08, 2023 at 03:57:23PM -0800, Wesley Cheng wrote: > Changes in v3: > - Changed prefix from RFC to PATCH > - Rebased entire series to usb-next > - Updated copyright years This is much better, thanks. I've done some very brief high-level skimming and found minor issues, and one major one. I'll let others review this in more depth right now, as I am way behind in reviewing other USB patches and need to get to them first. thanks, greg k-h
On 08/03/2023 23:57, Wesley Cheng wrote: > Q6DSP supports handling of USB playback audio data if USB audio offloading > is enabled. Add a new definition for the USB_RX AFE port, which is > referenced when the AFE port is started. > > Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> > Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com> Reviewed-by: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> > --- > include/dt-bindings/sound/qcom,q6dsp-lpass-ports.h | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/include/dt-bindings/sound/qcom,q6dsp-lpass-ports.h b/include/dt-bindings/sound/qcom,q6dsp-lpass-ports.h > index 9f7c5103bc82..746bc462bb2e 100644 > --- a/include/dt-bindings/sound/qcom,q6dsp-lpass-ports.h > +++ b/include/dt-bindings/sound/qcom,q6dsp-lpass-ports.h > @@ -131,6 +131,7 @@ > #define RX_CODEC_DMA_RX_7 126 > #define QUINARY_MI2S_RX 127 > #define QUINARY_MI2S_TX 128 > +#define USB_RX 129 > > #define LPASS_CLK_ID_PRI_MI2S_IBIT 1 > #define LPASS_CLK_ID_PRI_MI2S_EBIT 2
On 08/03/2023 23:57, Wesley Cheng wrote: > Create a USB BE component that will register a new USB port to the ASoC USB > framework. This will handle determination on if the requested audio > profile is supported by the USB device currently selected. > > Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com> Thanks Wesley for the patch, I have few minor comments. > --- > include/sound/q6usboffload.h | 20 ++++ > sound/soc/qcom/Kconfig | 4 + > sound/soc/qcom/qdsp6/Makefile | 1 + > sound/soc/qcom/qdsp6/q6usb.c | 208 ++++++++++++++++++++++++++++++++++ > 4 files changed, 233 insertions(+) > create mode 100644 include/sound/q6usboffload.h > create mode 100644 sound/soc/qcom/qdsp6/q6usb.c > > diff --git a/include/sound/q6usboffload.h b/include/sound/q6usboffload.h > new file mode 100644 > index 000000000000..4fb1912d9f55 > --- /dev/null > +++ b/include/sound/q6usboffload.h > @@ -0,0 +1,20 @@ > +/* SPDX-License-Identifier: GPL-2.0 > + * > + * linux/sound/q6usboffload.h -- QDSP6 USB offload > + * > + * Copyright (c) 2022-2023 Qualcomm Innovation Center, Inc. All rights reserved. > + */ > + > +/** > + * struct q6usb_offload > + * @dev - dev handle to usb be > + * @sid - streamID for iommu > + * @intr_num - usb interrupter number > + * @domain - allocated iommu domain > + **/ > +struct q6usb_offload { > + struct device *dev; > + long long sid; > + u32 intr_num; > + struct iommu_domain *domain; Why do we need to store this domain, You can remove this along with the one line that gets domain in probe function. > +}; > diff --git a/sound/soc/qcom/Kconfig b/sound/soc/qcom/Kconfig > index e7b00d1d9e99..bb285af6bb04 100644 > --- a/sound/soc/qcom/Kconfig > +++ b/sound/soc/qcom/Kconfig > @@ -114,6 +114,9 @@ config SND_SOC_QDSP6_APM > config SND_SOC_QDSP6_PRM_LPASS_CLOCKS > tristate > > +config SND_SOC_QDSP6_USB > + tristate > + > config SND_SOC_QDSP6_PRM > tristate > select SND_SOC_QDSP6_PRM_LPASS_CLOCKS > @@ -134,6 +137,7 @@ config SND_SOC_QDSP6 > select SND_SOC_TOPOLOGY > select SND_SOC_QDSP6_APM > select SND_SOC_QDSP6_PRM > + select SND_SOC_QDSP6_USB > help > To add support for MSM QDSP6 Soc Audio. > This will enable sound soc platform specific > diff --git a/sound/soc/qcom/qdsp6/Makefile b/sound/soc/qcom/qdsp6/Makefile > index 3963bf234664..c9457ee898d0 100644 > --- a/sound/soc/qcom/qdsp6/Makefile > +++ b/sound/soc/qcom/qdsp6/Makefile > @@ -17,3 +17,4 @@ obj-$(CONFIG_SND_SOC_QDSP6_APM_DAI) += q6apm-dai.o > obj-$(CONFIG_SND_SOC_QDSP6_APM_LPASS_DAI) += q6apm-lpass-dais.o > obj-$(CONFIG_SND_SOC_QDSP6_PRM) += q6prm.o > obj-$(CONFIG_SND_SOC_QDSP6_PRM_LPASS_CLOCKS) += q6prm-clocks.o > +obj-$(CONFIG_SND_SOC_QDSP6_USB) += q6usb.o > diff --git a/sound/soc/qcom/qdsp6/q6usb.c b/sound/soc/qcom/qdsp6/q6usb.c > new file mode 100644 > index 000000000000..3d480550b3f3 > --- /dev/null > +++ b/sound/soc/qcom/qdsp6/q6usb.c > @@ -0,0 +1,208 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (c) 2022-2023 Qualcomm Innovation Center, Inc. All rights reserved. > + */ > + > +#include <linux/err.h> > +#include <linux/init.h> > +#include <linux/module.h> > +#include <linux/device.h> > +#include <linux/platform_device.h> > +#include <linux/slab.h> > +#include <linux/iommu.h> > +#include <linux/dma-mapping.h> > +#include <linux/dma-map-ops.h> you would not need these 3 includes aswell. > + > +#include <sound/pcm.h> > +#include <sound/soc.h> > +#include <sound/soc-usb.h> > +#include <sound/pcm_params.h> > +#include <sound/asound.h> > +#include <sound/q6usboffload.h> > + > +#include "q6dsp-lpass-ports.h" > +#include "q6afe.h" > + > +#define SID_MASK 0xF > + > +struct q6usb_port_data { > + struct q6afe_usb_cfg usb_cfg; > + struct snd_soc_usb *usb; > + struct q6usb_offload priv; > + int active_idx; > +}; > + > +static const struct snd_soc_dapm_widget q6usb_dai_widgets[] = { > + SND_SOC_DAPM_HP("USB_RX_BE", NULL), > +}; > + > +static const struct snd_soc_dapm_route q6usb_dapm_routes[] = { > + {"USB Playback", NULL, "USB_RX_BE"}, > +}; > + > +static int q6usb_hw_params(struct snd_pcm_substream *substream, > + struct snd_pcm_hw_params *params, > + struct snd_soc_dai *dai) > +{ > + return 0; > +} > +static const struct snd_soc_dai_ops q6usb_ops = { > + .hw_params = q6usb_hw_params, > +}; > + > +static struct snd_soc_dai_driver q6usb_be_dais[] = { > + { > + .playback = { > + .stream_name = "USB BE RX", > + .rates = SNDRV_PCM_RATE_8000 | SNDRV_PCM_RATE_11025 | > + SNDRV_PCM_RATE_16000 | SNDRV_PCM_RATE_22050 | > + SNDRV_PCM_RATE_32000 | SNDRV_PCM_RATE_44100 | > + SNDRV_PCM_RATE_48000 | SNDRV_PCM_RATE_96000 | > + SNDRV_PCM_RATE_192000, > + .formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S16_BE | > + SNDRV_PCM_FMTBIT_U16_LE | SNDRV_PCM_FMTBIT_U16_BE | > + SNDRV_PCM_FMTBIT_S24_LE | SNDRV_PCM_FMTBIT_S24_BE | > + SNDRV_PCM_FMTBIT_U24_LE | SNDRV_PCM_FMTBIT_U24_BE, > + .channels_min = 1, > + .channels_max = 2, > + .rate_max = 192000, > + .rate_min = 8000, > + }, > + .id = USB_RX, > + .name = "USB_RX_BE", > + .ops = &q6usb_ops, > + }, > +}; > + > +static int q6usb_audio_ports_of_xlate_dai_name(struct snd_soc_component *component, > + const struct of_phandle_args *args, > + const char **dai_name) > +{ > + int id = args->args[0]; > + int ret = -EINVAL; > + int i; > + > + for (i = 0; i < ARRAY_SIZE(q6usb_be_dais); i++) { > + if (q6usb_be_dais[i].id == id) { > + *dai_name = q6usb_be_dais[i].name; > + ret = 0; > + break; > + } > + } > + > + return ret; > +} > + > +static int q6usb_alsa_connection_cb(struct snd_soc_usb *usb, int card_idx, > + int connected) > +{ > + struct snd_soc_dapm_context *dapm; > + struct q6usb_port_data *data; > + > + dapm = snd_soc_component_get_dapm(usb->component); > + data = dev_get_drvdata(usb->component->dev); > + > + if (connected) { > + snd_soc_dapm_enable_pin(dapm, "USB_RX_BE"); > + /* We only track the latest USB headset plugged in */ > + data->active_idx = card_idx; > + } else { > + snd_soc_dapm_disable_pin(dapm, "USB_RX_BE"); > + } > + snd_soc_dapm_sync(dapm); > + > + return 0; > +} > + > +static int q6usb_component_probe(struct snd_soc_component *component) > +{ > + struct q6usb_port_data *data = dev_get_drvdata(component->dev); > + struct snd_soc_dapm_context *dapm = snd_soc_component_get_dapm(component); > + > + snd_soc_dapm_disable_pin(dapm, "USB_RX_BE"); > + snd_soc_dapm_sync(dapm); > + > + data->usb = snd_soc_usb_add_port(component->dev, &data->priv, q6usb_alsa_connection_cb); > + if (IS_ERR(data->usb)) { > + dev_err(component->dev, "failed to add usb port\n"); > + return -ENODEV; > + } > + > + data->usb->component = component; > + > + return 0; > +} > + snd_soc_usb_remove_port should be part of component remove rather than platform device remove. static void q6usb_component_remove(struct snd_soc_component *component) { snd_soc_usb_remove_port(component->dev); } and delete q6usb_dai_dev_remove() > +static const struct snd_soc_component_driver q6usb_dai_component = { > + .probe = q6usb_component_probe, > + .name = "q6usb-dai-component", > + .dapm_widgets = q6usb_dai_widgets, > + .num_dapm_widgets = ARRAY_SIZE(q6usb_dai_widgets), > + .dapm_routes = q6usb_dapm_routes, > + .num_dapm_routes = ARRAY_SIZE(q6usb_dapm_routes), > + .of_xlate_dai_name = q6usb_audio_ports_of_xlate_dai_name, > +}; > + > +static int q6usb_dai_dev_probe(struct platform_device *pdev) > +{ > + struct device_node *node = pdev->dev.of_node; > + struct q6usb_port_data *data; > + struct device *dev = &pdev->dev; > + struct of_phandle_args args; > + int ret; > + > + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL); > + if (!data) > + return -ENOMEM; > + > + ret = of_property_read_u32(node, "qcom,usb-audio-intr-num", > + &data->priv.intr_num); > + if (ret) { > + dev_err(&pdev->dev, "failed to read intr num.\n"); > + return ret; > + } > + > + ret = of_parse_phandle_with_fixed_args(node, "iommus", 1, 0, &args); > + if (ret < 0) > + data->priv.sid = -1; > + else > + data->priv.sid = args.args[0] & SID_MASK; > + > + data->priv.domain = iommu_get_domain_for_dev(&pdev->dev); > + > + data->priv.dev = dev; > + dev_set_drvdata(dev, data); > + > + ret = devm_snd_soc_register_component(dev, &q6usb_dai_component, > + q6usb_be_dais, ARRAY_SIZE(q6usb_be_dais)); > + if (ret < 0) > + return ret; > + > + return 0; > +} > + > +static int q6usb_dai_dev_remove(struct platform_device *pdev) > +{ > + snd_soc_usb_remove_port(&pdev->dev); > + > + return 0; > +} > + > +static const struct of_device_id q6usb_dai_device_id[] = { > + { .compatible = "qcom,q6usb-dais" }, > + {}, > +}; > +MODULE_DEVICE_TABLE(of, q6usb_dai_device_id); > + > +static struct platform_driver q6usb_dai_platform_driver = { > + .driver = { > + .name = "q6usb-dai", > + .of_match_table = of_match_ptr(q6usb_dai_device_id), > + }, > + .probe = q6usb_dai_dev_probe, > + .remove = q6usb_dai_dev_remove, > +}; > +module_platform_driver(q6usb_dai_platform_driver); > + > +MODULE_DESCRIPTION("Q6 USB backend dai driver"); > +MODULE_LICENSE("GPL");
On 3/8/23 17:57, Wesley Cheng wrote: > Changes in v3: > - Changed prefix from RFC to PATCH > - Rebased entire series to usb-next > - Updated copyright years You may want to move the per-version changes after the description of this patchset. > XHCI: > - Rebased changes on top of XHCI changes merged into usb-next, and only added > changes that were still under discussion. > - Added change to read in the "num-hc-interrupters" device property. > > ASoC: > - qusb6 USB backend > - Incorporated suggestions to fetch iommu information with existing APIs > - Added two new sound kcontrols to fetch offload status and offload device > selection. > - offload status - will return the card and pcm device in use > tinymix -D 0 get 1 --> 1, 0 (offload in progress on card#1 pcm#0) > > - device selection - set the card and pcm device to enable offload on. Ex.: > tinymix -D 0 set 1 2 0 --> sets offload on card#2 pcm#0 > (this should be the USB card) > > USB SND: > - Fixed up some locking related concerns for registering platform ops. > - Moved callbacks under the register_mutex, so that > - Modified APIs to properly pass more information about the USB SND device, so > that the Q6USB backend can build a device list/map, in order to monitor offload > status and device selection. > > Changes in v2: > > XHCI: > - Replaced XHCI and HCD changes with Mathias' XHCI interrupter changes > in his tree: > https://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git/log/?h=feature_interrupters > > Adjustments made to Mathias' changes: > - Created xhci-intr.h to export/expose interrupter APIs versus exposing xhci.h. > Moved dependent structures to this file as well. (so clients can parse out > information from "struct xhci_interrupter") > - Added some basic locking when requesting interrupters. > - Fixed up some sanity checks. > - Removed clearing of the ERSTBA during freeing of the interrupter. (pending > issue where SMMU fault occurs if DMA addr returned is 64b - TODO) > > - Clean up pending events in the XHCI secondary interrupter. While testing USB > bus suspend, it was seen that on bus resume, the xHCI HC would run into a command > timeout. > - Added offloading APIs to xHCI to fetch transfer and event ring information. > > ASoC: > - Modified soc-usb to allow for multiple USB port additions. For this to work, > the USB offload driver has to have a reference to the USB backend by adding > a "usb-soc-be" DT entry to the device saved into XHCI sysdev. > - Created separate dt-bindings for defining USB_RX port. > - Increased APR timeout to accommodate the situation where the AFE port start > command could be delayed due to having to issue a USB bus resume while > handling the QMI stream start command. > > USB SND: > - Added a platform ops during usb_audio_suspend(). This allows for the USB > offload driver to halt the audio stream when system enters PM suspend. This > ensures the audio DSP is not issuing transfers on the USB bus. > - Do not override platform ops if they are already populated. > - Introduce a shared status variable between the USB offload and USB SND layers, > to ensure that only one path is active at a time. If the USB bus is occupied, > then userspace is notified that the path is busy. ---> start of the cover letter: > Several Qualcomm based chipsets can support USB audio offloading to a > dedicated audio DSP, which can take over issuing transfers to the USB > host controller. The intention is to reduce the load on the main > processors in the SoC, and allow them to be placed into lower power modes. > There are several parts to this design: > 1. Adding ASoC binding layer > 2. Create a USB backend for Q6DSP clarify what 'backend' means. I would guess you are referring to the ASoC DPCM concept of Back-End, which typically exposes an audio interface controlled by a DSP? If yes, the description should be "Create a Q6DSP ASoC DPCM BackEnd for USB audio playback/record". > 3. Introduce XHCI interrupter support > 4. Create vendor ops for the USB SND driver Add an introduction as to why this would be needed? which 'vendor' are we talking about, is this on the SOC side or the USB device side? > Adding ASoC binding layer: > soc-usb: Intention is to treat a USB port similar to a headphone jack. > The port is always present on the device, but cable/pin status can be > enabled/disabled. Expose mechanisms for USB backend ASoC drivers to > communicate with USB SND. port == backend? Using different words for the same concept is a sure way to lose the reviewers, and not describing different concepts isn't much better. > Create a USB backend for Q6DSP: > q6usb: Basic backend driver that will be responsible for maintaining the What does this mean? Is this a module that registers a set of ASoC DAIs? It's not clear to me what a 'backend driver' means either in a USB or ASoC context. > resources needed to initiate a playback stream using the Q6DSP. Will > be the entity that checks to make sure the connected USB audio device > supports the requested PCM format. If it does not, the PCM open call will > fail, and userpsace ALSA can take action accordingly. If it's an ASoC backend, there is no PCM open call. You're probably referring to the hw_params, not the open/startup anyways. > > Introduce XHCI interrupter support: > XHCI HCD supports multiple interrupters, which allows for events to be routed > to different event rings. This is determined by "Interrupter Target" field > specified in Section "6.4.1.1 Normal TRB" of the XHCI specification. > > Events in the offloading case will be routed to an event ring that is assigned > to the audio DSP. > > Create vendor ops for the USB SND driver: > qc_audio_offload: This particular driver has several components associated > with it: > - QMI stream request handler > - XHCI interrupter and resource management > - audio DSP memory management so how does this 'qc_audio_offload' interface with 'q6usb' described above? how are the roles different or complementary? > When the audio DSP wants to enable a playback stream, the request is first > received by the ASoC platform sound card. Depending on the selected route, > ASoC will bring up the individual DAIs in the path. The Q6USB backend DAI > will send an AFE port start command (with enabling the USB playback path), and > the audio DSP will handle the request accordingly. > > Part of the AFE USB port start handling will have an exchange of control > messages using the QMI protocol. The qc_audio_offload driver will populate the > buffer information: > - Event ring base address > - EP transfer ring base address > > and pass it along to the audio DSP. All endpoint management will now be handed > over to the DSP, and the main processor is not involved in transfers. > > Overall, implementing this feature will still expose separate sound card and PCM > devices for both the platorm card and USB audio device: > 0 [SM8250MTPWCD938]: sm8250 - SM8250-MTP-WCD9380-WSA8810-VA-D > SM8250-MTP-WCD9380-WSA8810-VA-DMIC > 1 [Audio ]: USB-Audio - USB Audio > Generic USB Audio at usb-xhci-hcd.1.auto-1.4, high speed > > This is to ensure that userspace ALSA entities can decide which route to take > when executing the audio playback. In the above, if card#1 is selected, then > USB audio data will take the legacy path over the USB PCM drivers, etc... I already voiced my concerns about exposing two cards, each with their own set of volume controls with the same device. It would be much better to have an additional offloaded PCM device for card0... But if the consensus is to have two cards, it's still not clear how the routing would be selected. In the case where there are two USB audio devices attached, the offloaded path would only support one of the two. How would userspace know which of the two is selected? And how would userspace know the difference anyways between two physical devices attached to the platform with no offload, and one physical device with one additional offload path? The names you selected can't be used to identify that card1 is the optimized version of card0. Before we review low-level kernel plumbing, it would be good to give a better overview of how userspace applications are supposed to interact with the cards and identify the offloaded path. Testing with tinyplay/tinymix is fine, but that's a developer-level or CI unit test. we've got to see the broader picture of how a sound server would use this USB offload capability.
> +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, > + }, > +};
Hi Srinivas, On 3/9/2023 1:01 AM, Srinivas Kandagatla wrote: > > > On 08/03/2023 23:57, Wesley Cheng wrote: >> Create a USB BE component that will register a new USB port to the >> ASoC USB >> framework. This will handle determination on if the requested audio >> profile is supported by the USB device currently selected. >> >> Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com> > > Thanks Wesley for the patch, I have few minor comments. > Thanks for the review! >> --- >> include/sound/q6usboffload.h | 20 ++++ >> sound/soc/qcom/Kconfig | 4 + >> sound/soc/qcom/qdsp6/Makefile | 1 + >> sound/soc/qcom/qdsp6/q6usb.c | 208 ++++++++++++++++++++++++++++++++++ >> 4 files changed, 233 insertions(+) >> create mode 100644 include/sound/q6usboffload.h >> create mode 100644 sound/soc/qcom/qdsp6/q6usb.c >> >> diff --git a/include/sound/q6usboffload.h b/include/sound/q6usboffload.h >> new file mode 100644 >> index 000000000000..4fb1912d9f55 >> --- /dev/null >> +++ b/include/sound/q6usboffload.h >> @@ -0,0 +1,20 @@ >> +/* SPDX-License-Identifier: GPL-2.0 >> + * >> + * linux/sound/q6usboffload.h -- QDSP6 USB offload >> + * >> + * Copyright (c) 2022-2023 Qualcomm Innovation Center, Inc. All >> rights reserved. >> + */ >> + >> +/** >> + * struct q6usb_offload >> + * @dev - dev handle to usb be >> + * @sid - streamID for iommu >> + * @intr_num - usb interrupter number >> + * @domain - allocated iommu domain >> + **/ >> +struct q6usb_offload { >> + struct device *dev; >> + long long sid; >> + u32 intr_num; >> + struct iommu_domain *domain; > Why do we need to store this domain, You can remove this along with the > one line that gets domain in probe function. > We'll need a reference to the iommu domain, because the QC USB offload driver will be the one that is going to map the XHCI interrupter and transfer ring regions for the audio DSP. This happens when a USB QMI enable stream request is received in the USB offload driver. Please refer to: static int prepare_qmi_response(struct snd_usb_substream *subs, struct qmi_uaudio_stream_req_msg_v01 *req_msg, struct qmi_uaudio_stream_resp_msg_v01 *resp, int info_idx) { ... xhci_pa = xhci_get_ir_resource(subs->dev, ir); if (!xhci_pa) { dev_err(uaudio_qdev->dev, "failed to get sec event ring address\n"); ret = -ENODEV; goto free_sec_ring; } ... va = uaudio_iommu_map(MEM_EVENT_RING, dma_coherent, xhci_pa, PAGE_SIZE, NULL); if (!va) { ret = -ENOMEM; goto free_sec_ring; } This is just an example for mapping the XHCI secondary interrupter. We will also do the same for the transfer ring. Thanks Wesley Cheng
Hi Pierre, On 3/9/2023 9:13 AM, Pierre-Louis Bossart wrote: > > > On 3/8/23 17:57, Wesley Cheng wrote: >> Changes in v3: >> - Changed prefix from RFC to PATCH >> - Rebased entire series to usb-next >> - Updated copyright years > > You may want to move the per-version changes after the description of > this patchset. > Thanks for reading through this :). Sure, will do that going forward. >> XHCI: >> - Rebased changes on top of XHCI changes merged into usb-next, and only added >> changes that were still under discussion. >> - Added change to read in the "num-hc-interrupters" device property. >> >> ASoC: >> - qusb6 USB backend >> - Incorporated suggestions to fetch iommu information with existing APIs >> - Added two new sound kcontrols to fetch offload status and offload device >> selection. >> - offload status - will return the card and pcm device in use >> tinymix -D 0 get 1 --> 1, 0 (offload in progress on card#1 pcm#0) >> >> - device selection - set the card and pcm device to enable offload on. Ex.: >> tinymix -D 0 set 1 2 0 --> sets offload on card#2 pcm#0 >> (this should be the USB card) >> >> USB SND: >> - Fixed up some locking related concerns for registering platform ops. >> - Moved callbacks under the register_mutex, so that >> - Modified APIs to properly pass more information about the USB SND device, so >> that the Q6USB backend can build a device list/map, in order to monitor offload >> status and device selection. >> >> Changes in v2: >> >> XHCI: >> - Replaced XHCI and HCD changes with Mathias' XHCI interrupter changes >> in his tree: >> https://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git/log/?h=feature_interrupters >> >> Adjustments made to Mathias' changes: >> - Created xhci-intr.h to export/expose interrupter APIs versus exposing xhci.h. >> Moved dependent structures to this file as well. (so clients can parse out >> information from "struct xhci_interrupter") >> - Added some basic locking when requesting interrupters. >> - Fixed up some sanity checks. >> - Removed clearing of the ERSTBA during freeing of the interrupter. (pending >> issue where SMMU fault occurs if DMA addr returned is 64b - TODO) >> >> - Clean up pending events in the XHCI secondary interrupter. While testing USB >> bus suspend, it was seen that on bus resume, the xHCI HC would run into a command >> timeout. >> - Added offloading APIs to xHCI to fetch transfer and event ring information. >> >> ASoC: >> - Modified soc-usb to allow for multiple USB port additions. For this to work, >> the USB offload driver has to have a reference to the USB backend by adding >> a "usb-soc-be" DT entry to the device saved into XHCI sysdev. >> - Created separate dt-bindings for defining USB_RX port. >> - Increased APR timeout to accommodate the situation where the AFE port start >> command could be delayed due to having to issue a USB bus resume while >> handling the QMI stream start command. >> >> USB SND: >> - Added a platform ops during usb_audio_suspend(). This allows for the USB >> offload driver to halt the audio stream when system enters PM suspend. This >> ensures the audio DSP is not issuing transfers on the USB bus. >> - Do not override platform ops if they are already populated. >> - Introduce a shared status variable between the USB offload and USB SND layers, >> to ensure that only one path is active at a time. If the USB bus is occupied, >> then userspace is notified that the path is busy. > > > ---> start of the cover letter: > >> Several Qualcomm based chipsets can support USB audio offloading to a >> dedicated audio DSP, which can take over issuing transfers to the USB >> host controller. The intention is to reduce the load on the main >> processors in the SoC, and allow them to be placed into lower power modes. >> There are several parts to this design: >> 1. Adding ASoC binding layer >> 2. Create a USB backend for Q6DSP > > clarify what 'backend' means. I would guess you are referring to the > ASoC DPCM concept of Back-End, which typically exposes an audio > interface controlled by a DSP? > > If yes, the description should be > > "Create a Q6DSP ASoC DPCM BackEnd for USB audio playback/record". > You're correct. "backend" was referring to a Q6DSP ASoC DPCM backend. >> 3. Introduce XHCI interrupter support >> 4. Create vendor ops for the USB SND driver > > Add an introduction as to why this would be needed? which 'vendor' are > we talking about, is this on the SOC side or the USB device side? > This would be referring to the SoC side. For example, QCOM can potentially have a different implementation of offloading USB SND to the audio DSP versus another OEM. This is to allow those other entities to be able to define their own offload methodology. For the most part, the common operations across all offloading implementations have so far is being able to handle USB SND connect/disconnect events. >> Adding ASoC binding layer: >> soc-usb: Intention is to treat a USB port similar to a headphone jack. >> The port is always present on the device, but cable/pin status can be >> enabled/disabled. Expose mechanisms for USB backend ASoC drivers to >> communicate with USB SND. > > port == backend? > > Using different words for the same concept is a sure way to lose the > reviewers, and not describing different concepts isn't much better. > Yes, port is corresponding to the >> Create a USB backend for Q6DSP: >> q6usb: Basic backend driver that will be responsible for maintaining the > > What does this mean? Is this a module that registers a set of ASoC DAIs? > > It's not clear to me what a 'backend driver' means either in a USB or > ASoC context. > Will fix this up next time. It refers to a Q6DSP DPCM ASoC backend, as you mentioned above. >> resources needed to initiate a playback stream using the Q6DSP. Will >> be the entity that checks to make sure the connected USB audio device >> supports the requested PCM format. If it does not, the PCM open call will >> fail, and userpsace ALSA can take action accordingly. > > If it's an ASoC backend, there is no PCM open call. You're probably > referring to the hw_params, not the open/startup anyways. > Correct. Will fix this. >> >> Introduce XHCI interrupter support: >> XHCI HCD supports multiple interrupters, which allows for events to be routed >> to different event rings. This is determined by "Interrupter Target" field >> specified in Section "6.4.1.1 Normal TRB" of the XHCI specification. >> >> Events in the offloading case will be routed to an event ring that is assigned >> to the audio DSP. >> >> Create vendor ops for the USB SND driver: >> qc_audio_offload: This particular driver has several components associated >> with it: >> - QMI stream request handler >> - XHCI interrupter and resource management >> - audio DSP memory management > > so how does this 'qc_audio_offload' interface with 'q6usb' described > above? how are the roles different or complementary? > So in general you can think that the qc_audio_offload is a complement to the USB SND USB class driver, while q6usb is to ASoC. Since the ASoC framework doesn't have any communication with USB SND, the ASoC DPCM USB backend (q6usb) will have to be the entity that maintains what is going on in USB SND. That way, sessions initiated through the ASoC managed sound card can evaluate what is available based on information reported by q6usb. qc_audio_offload and q6usb will have some interaction between each other. The majority of communication between qc_audio_offload and q6usb is reporting the device connection events. >> When the audio DSP wants to enable a playback stream, the request is first >> received by the ASoC platform sound card. Depending on the selected route, >> ASoC will bring up the individual DAIs in the path. The Q6USB backend DAI >> will send an AFE port start command (with enabling the USB playback path), and >> the audio DSP will handle the request accordingly. >> >> Part of the AFE USB port start handling will have an exchange of control >> messages using the QMI protocol. The qc_audio_offload driver will populate the >> buffer information: >> - Event ring base address >> - EP transfer ring base address >> >> and pass it along to the audio DSP. All endpoint management will now be handed >> over to the DSP, and the main processor is not involved in transfers. >> >> Overall, implementing this feature will still expose separate sound card and PCM >> devices for both the platorm card and USB audio device: >> 0 [SM8250MTPWCD938]: sm8250 - SM8250-MTP-WCD9380-WSA8810-VA-D >> SM8250-MTP-WCD9380-WSA8810-VA-DMIC >> 1 [Audio ]: USB-Audio - USB Audio >> Generic USB Audio at usb-xhci-hcd.1.auto-1.4, high speed >> >> This is to ensure that userspace ALSA entities can decide which route to take >> when executing the audio playback. In the above, if card#1 is selected, then >> USB audio data will take the legacy path over the USB PCM drivers, etc... > > I already voiced my concerns about exposing two cards, each with their > own set of volume controls with the same device. It would be much better > to have an additional offloaded PCM device for card0... > > But if the consensus is to have two cards, it's still not clear how the > routing would be selected. In the case where there are two USB audio > devices attached, the offloaded path would only support one of the two. > How would userspace know which of the two is selected? > With patch#24: https://lore.kernel.org/linux-usb/20230308235751.495-25-quic_wcheng@quicinc.com/T/#u Now, userspace can at least choose which device it wants to offload. Part of doing that would mean userspace knows what USB SND card devices are available, so it is aware of which devices are shared (between the offload and USB SND path) > And how would userspace know the difference anyways between two physical > devices attached to the platform with no offload, and one physical > device with one additional offload path? The names you selected can't be > used to identify that card1 is the optimized version of card0. > Is userspace currently able to differentiate between cards that are created by USB SND versus ASoC? How complex can the userspace card discovery be? Can it query kcontrols at this point in time? If so, maybe we can change the names of the newly added ones to reflect that it is an offload device? SND kcontrol names are currently: Q6USB offload status Q6USB offload SND device select > Before we review low-level kernel plumbing, it would be good to give a > better overview of how userspace applications are supposed to interact > with the cards and identify the offloaded path. Testing with > tinyplay/tinymix is fine, but that's a developer-level or CI unit test. > we've got to see the broader picture of how a sound server would use > this USB offload capability. Sure, I think that is fine. I was hoping that at least adding some of the new kcontrols would help userspace make use of this path in general, but we can add more information if required. Thanks Wesley Cheng
>>> Create vendor ops for the USB SND driver: >>> qc_audio_offload: This particular driver has several components >>> associated >>> with it: >>> - QMI stream request handler >>> - XHCI interrupter and resource management >>> - audio DSP memory management >> >> so how does this 'qc_audio_offload' interface with 'q6usb' described >> above? how are the roles different or complementary? >> > So in general you can think that the qc_audio_offload is a complement to > the USB SND USB class driver, while q6usb is to ASoC. Since the ASoC Humm, that is far from clear. I don't get how a something that interacts with the USB class driver can also be in charge of the audio DSP memory management. > framework doesn't have any communication with USB SND, the ASoC DPCM USB > backend (q6usb) will have to be the entity that maintains what is going > on in USB SND. That way, sessions initiated through the ASoC managed > sound card can evaluate what is available based on information reported > by q6usb. > > qc_audio_offload and q6usb will have some interaction between each > other. The majority of communication between qc_audio_offload and q6usb > is reporting the device connection events. It's already complicated to figure out how the DSP and USB class driver might interact and probe/timing dependencies, but with two additional drivers in the mix it's really hard to understand. Maybe ascii-art would help describe the concepts and types of information exchanged. Maintaining a consistent state across multiple drivers is not an easy task. > >>> When the audio DSP wants to enable a playback stream, the request is >>> first >>> received by the ASoC platform sound card. Depending on the selected >>> route, >>> ASoC will bring up the individual DAIs in the path. The Q6USB >>> backend DAI >>> will send an AFE port start command (with enabling the USB playback >>> path), and >>> the audio DSP will handle the request accordingly. >>> >>> Part of the AFE USB port start handling will have an exchange of control >>> messages using the QMI protocol. The qc_audio_offload driver will >>> populate the >>> buffer information: >>> - Event ring base address >>> - EP transfer ring base address >>> >>> and pass it along to the audio DSP. All endpoint management will now >>> be handed >>> over to the DSP, and the main processor is not involved in transfers. >>> >>> Overall, implementing this feature will still expose separate sound >>> card and PCM >>> devices for both the platorm card and USB audio device: >>> 0 [SM8250MTPWCD938]: sm8250 - SM8250-MTP-WCD9380-WSA8810-VA-D >>> SM8250-MTP-WCD9380-WSA8810-VA-DMIC >>> 1 [Audio ]: USB-Audio - USB Audio >>> Generic USB Audio at usb-xhci-hcd.1.auto-1.4, >>> high speed >>> >>> This is to ensure that userspace ALSA entities can decide which route >>> to take >>> when executing the audio playback. In the above, if card#1 is >>> selected, then >>> USB audio data will take the legacy path over the USB PCM drivers, >>> etc... >> >> I already voiced my concerns about exposing two cards, each with their >> own set of volume controls with the same device. It would be much better >> to have an additional offloaded PCM device for card0... >> >> But if the consensus is to have two cards, it's still not clear how the >> routing would be selected. In the case where there are two USB audio >> devices attached, the offloaded path would only support one of the two. >> How would userspace know which of the two is selected? >> > > With patch#24: > https://lore.kernel.org/linux-usb/20230308235751.495-25-quic_wcheng@quicinc.com/T/#u > > Now, userspace can at least choose which device it wants to offload. > Part of doing that would mean userspace knows what USB SND card devices > are available, so it is aware of which devices are shared (between the > offload and USB SND path) > >> And how would userspace know the difference anyways between two physical >> devices attached to the platform with no offload, and one physical >> device with one additional offload path? The names you selected can't be >> used to identify that card1 is the optimized version of card0. >> > > Is userspace currently able to differentiate between cards that are > created by USB SND versus ASoC? How complex can the userspace card > discovery be? Can it query kcontrols at this point in time? If so, > maybe we can change the names of the newly added ones to reflect that it > is an offload device? > > SND kcontrol names are currently: > Q6USB offload status > Q6USB offload SND device select I must admit I've never seen kcontrols being used to identify what the card is, and in this case it's a pretend-card that's just an improved version of another. It might be easier to use something else, such as the component strings. > >> Before we review low-level kernel plumbing, it would be good to give a >> better overview of how userspace applications are supposed to interact >> with the cards and identify the offloaded path. Testing with >> tinyplay/tinymix is fine, but that's a developer-level or CI unit test. >> we've got to see the broader picture of how a sound server would use >> this USB offload capability. > > Sure, I think that is fine. I was hoping that at least adding some of > the new kcontrols would help userspace make use of this path in general, > but we can add more information if required. Can I ask if this solution has been used with a complete userspace stack already? I could see how this might be used with a relatively fixed Android HAL, where the platform and routing are relatively controlled. I don't see how a more generic audio server would deal with the discovery and routing.
On 09/03/2023 19:38, Wesley Cheng wrote: > Hi Srinivas, > > On 3/9/2023 1:01 AM, Srinivas Kandagatla wrote: >> >> >> On 08/03/2023 23:57, Wesley Cheng wrote: >>> Create a USB BE component that will register a new USB port to the >>> ASoC USB >>> framework. This will handle determination on if the requested audio >>> profile is supported by the USB device currently selected. >>> >>> Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com> >> >> Thanks Wesley for the patch, I have few minor comments. >> > > Thanks for the review! > >>> --- >>> include/sound/q6usboffload.h | 20 ++++ >>> sound/soc/qcom/Kconfig | 4 + >>> sound/soc/qcom/qdsp6/Makefile | 1 + >>> sound/soc/qcom/qdsp6/q6usb.c | 208 ++++++++++++++++++++++++++++++++++ >>> 4 files changed, 233 insertions(+) >>> create mode 100644 include/sound/q6usboffload.h >>> create mode 100644 sound/soc/qcom/qdsp6/q6usb.c >>> >>> diff --git a/include/sound/q6usboffload.h b/include/sound/q6usboffload.h >>> new file mode 100644 >>> index 000000000000..4fb1912d9f55 >>> --- /dev/null >>> +++ b/include/sound/q6usboffload.h >>> @@ -0,0 +1,20 @@ >>> +/* SPDX-License-Identifier: GPL-2.0 >>> + * >>> + * linux/sound/q6usboffload.h -- QDSP6 USB offload >>> + * >>> + * Copyright (c) 2022-2023 Qualcomm Innovation Center, Inc. All >>> rights reserved. >>> + */ >>> + >>> +/** >>> + * struct q6usb_offload >>> + * @dev - dev handle to usb be >>> + * @sid - streamID for iommu >>> + * @intr_num - usb interrupter number >>> + * @domain - allocated iommu domain >>> + **/ >>> +struct q6usb_offload { >>> + struct device *dev; >>> + long long sid; >>> + u32 intr_num; >>> + struct iommu_domain *domain; >> Why do we need to store this domain, You can remove this along with >> the one line that gets domain in probe function. >> > > We'll need a reference to the iommu domain, because the QC USB offload > driver will be the one that is going to map the XHCI interrupter and > transfer ring regions for the audio DSP. This happens when a USB QMI this is okay, AFAIU, as long as uaudio_qdev->dev pointer is used in dma alloc apis like dma_map*, dma_alloc_* you would not need to handle iommu_domain directly like this in drivers. --srini > enable stream request is received in the USB offload driver. Please > refer to: > > static int prepare_qmi_response(struct snd_usb_substream *subs, > struct qmi_uaudio_stream_req_msg_v01 *req_msg, > struct qmi_uaudio_stream_resp_msg_v01 *resp, int info_idx) > { > ... > xhci_pa = xhci_get_ir_resource(subs->dev, ir); > if (!xhci_pa) { > dev_err(uaudio_qdev->dev, > "failed to get sec event ring address\n"); > ret = -ENODEV; > goto free_sec_ring; > } > ... > va = uaudio_iommu_map(MEM_EVENT_RING, dma_coherent, xhci_pa, > PAGE_SIZE, > NULL); > if (!va) { > ret = -ENOMEM; > goto free_sec_ring; > } > > This is just an example for mapping the XHCI secondary interrupter. We > will also do the same for the transfer ring. > > Thanks > Wesley Cheng
Hi Pierre, On 3/9/2023 4:37 PM, Pierre-Louis Bossart wrote: > >>>> Create vendor ops for the USB SND driver: >>>> qc_audio_offload: This particular driver has several components >>>> associated >>>> with it: >>>> - QMI stream request handler >>>> - XHCI interrupter and resource management >>>> - audio DSP memory management >>> >>> so how does this 'qc_audio_offload' interface with 'q6usb' described >>> above? how are the roles different or complementary? >>> >> So in general you can think that the qc_audio_offload is a complement to >> the USB SND USB class driver, while q6usb is to ASoC. Since the ASoC > > Humm, that is far from clear. I don't get how a something that interacts > with the USB class driver can also be in charge of the audio DSP memory > management. > This is because the USB class driver is the entity which is going to work with the USB HCD (XHCI) in this case to fetch the required addresses, and map that into memory accessible by the audio DSP. It would be odd to be doing that from the q6usb end, which is part of the ASoC layer. >> framework doesn't have any communication with USB SND, the ASoC DPCM USB >> backend (q6usb) will have to be the entity that maintains what is going >> on in USB SND. That way, sessions initiated through the ASoC managed >> sound card can evaluate what is available based on information reported >> by q6usb. >> >> qc_audio_offload and q6usb will have some interaction between each >> other. The majority of communication between qc_audio_offload and q6usb >> is reporting the device connection events. > > It's already complicated to figure out how the DSP and USB class driver > might interact and probe/timing dependencies, but with two additional > drivers in the mix it's really hard to understand. > I did test some cases based on existence of both these drivers (qc_audio_offload and q6usb). If either one doesn't exist in the system, then the offload path would not work. I did improve some of these potential sequences in the latest revision, such as patch#28. This would address scenarios where the q6usb ASoC DPCM backend wasn't probed, while the USB SND (and qc_audio_offload) were still detecting device connections. Once the Q6USB driver is probed, then the offload snd kcontrols would be created, and devices would be properly identified with the rediscover api. > Maybe ascii-art would help describe the concepts and types of > information exchanged. Maintaining a consistent state across multiple > drivers is not an easy task. > Hopefully this might help? I know its a lot to read through. USB | ASoC -------------------------------------------------------------------- | _________________________ | |sm8250 platform card | | |_________________________| | | | | ___V____ ____V____ | |Q6USB | |Q6AFE | #5 | |"codec" | |"cpu" | | |________| |_________| | ^ | | #6 | ___V____ | |SOC-USB | ________ #1 ________ #7 | | |USB SND |<--->|QC offld|<------------>|________| |(card.c)| | |<---------- ^ |________| |________|___ #4 | | | ^ ^ | | | ___V__________________ | #2 | #2 | | | |APR/GLINK | __ V_______________V_____ | | | |______________________| |USB SND (endpoint.c) | | | | ^ |_________________________| | | | #8 | ^ | | | ___________V___________ | #3 | | |->|audio DSP | ___________V_____________ | | |_______________________| |XHCI HCD |<- | |_________________________| | #1 - USB SND and QC offload: Initialization: - Register platform operations, to receive connect/disconnect events from USB SND. - QC offload creates a QMI handle, in order to receive QMI requests from the audio DSP. Runtime: - USB SND passes along "struct snd_usb_audio" in order for QC offload to reference USB UAC desc parsing/USB SND helper APIs. - USB device disconnection events will result in clearing of the chip entry. #2 - USB SND and QC offload endpoints: Runtime: - In the non-offloaded path, USB snd will utilize functions exposed by USB SND endpoint, to help with fetching USB EP references and queuing URBs. - In the offload path, qc offload will utilize the functions to fetch USB EP references, so that it can use that information to query the XHCI HCD. - Likewise, both will clean up endpoints when audio stream is not in use. #3 - XHCI HCD: Initialization: - During XHCI probe timing, when the USB HCD is added to the system, it will also initialize the secondary event rings. Runtime: - During USB device plug ins/outs, allocates device slot, assigns eps, and initializes transfer rings. #4 - QC offload and XHCI: Runtime: - QC offload needs to reference the transfer ring and secondary event ring addresses by executing XHCI offload management APIs. - This happens when audio DSP receives a USB QMI stream request. #5 - ASoC components: Initialization: - The SM8250 platform sound card driver fetches DT node entries defining the ASoC links. This chain/link has the components involved for a particular Q6AFE path. (not only USB offload) - "cpu" - this is the ASoC CPU DAI that handles interaction with the Q6 DSP's audio protocol. (AFE ports) - "codec" - the ASoC codec (backend) DAI defined - Registers ASoC platform sound card based on links defined in the DT node. - Probes DAI components involved, ie Q6USB and Q6AFE Runtime: - Q6AFE has the bulk of the interaction w/ the audio DSP to start an audio session, such as issuing AFE port start commands (part of the protocol used to communicate the audio session info) - Q6USB will be there to now check for if format requested is supported by the device, and maintain offloading status. #6 - Q6USB and SOC-USB: Initialization: - Q6USB will query QC offload for USB device connection states. (through soc-usb) - Creates a SOC USB entry, that carries information about resources, such as audio DSP memory information and requested XHCI event ring index. Runtime: - SOC-USB will receive connect/disconnect events and propagate to Q6USB. - Q6USB makes devices available for offloading based on these events. - Sets Q6AFE port configurations to select the USB SND card# and PCM#. #7 - SOC-USB and QC offload: Initialization: - Rediscover USB SND devices when the SOC-USB entry is created (if needed) - For situations where the Q6USB DAI hasn't been probed. Runtime: - Propagate connect/disconnect events. #8 - audio DSP and QC offload: Runtime: - Handle QMI requests coming from audio DSP. These requests come AFTER the Q6AFE port is opened by the Q6AFE DAI(#6) - Returns memory information about resources allocated by XHCI. - Enables audio playback when this QMI transaction is completed. >> >>>> When the audio DSP wants to enable a playback stream, the request is >>>> first >>>> received by the ASoC platform sound card. Depending on the selected >>>> route, >>>> ASoC will bring up the individual DAIs in the path. The Q6USB >>>> backend DAI >>>> will send an AFE port start command (with enabling the USB playback >>>> path), and >>>> the audio DSP will handle the request accordingly. >>>> >>>> Part of the AFE USB port start handling will have an exchange of control >>>> messages using the QMI protocol. The qc_audio_offload driver will >>>> populate the >>>> buffer information: >>>> - Event ring base address >>>> - EP transfer ring base address >>>> >>>> and pass it along to the audio DSP. All endpoint management will now >>>> be handed >>>> over to the DSP, and the main processor is not involved in transfers. >>>> >>>> Overall, implementing this feature will still expose separate sound >>>> card and PCM >>>> devices for both the platorm card and USB audio device: >>>> 0 [SM8250MTPWCD938]: sm8250 - SM8250-MTP-WCD9380-WSA8810-VA-D >>>> SM8250-MTP-WCD9380-WSA8810-VA-DMIC >>>> 1 [Audio ]: USB-Audio - USB Audio >>>> Generic USB Audio at usb-xhci-hcd.1.auto-1.4, >>>> high speed >>>> >>>> This is to ensure that userspace ALSA entities can decide which route >>>> to take >>>> when executing the audio playback. In the above, if card#1 is >>>> selected, then >>>> USB audio data will take the legacy path over the USB PCM drivers, >>>> etc... >>> >>> I already voiced my concerns about exposing two cards, each with their >>> own set of volume controls with the same device. It would be much better >>> to have an additional offloaded PCM device for card0... >>> >>> But if the consensus is to have two cards, it's still not clear how the >>> routing would be selected. In the case where there are two USB audio >>> devices attached, the offloaded path would only support one of the two. >>> How would userspace know which of the two is selected? >>> >> >> With patch#24: >> https://lore.kernel.org/linux-usb/20230308235751.495-25-quic_wcheng@quicinc.com/T/#u >> >> Now, userspace can at least choose which device it wants to offload. >> Part of doing that would mean userspace knows what USB SND card devices >> are available, so it is aware of which devices are shared (between the >> offload and USB SND path) >> >>> And how would userspace know the difference anyways between two physical >>> devices attached to the platform with no offload, and one physical >>> device with one additional offload path? The names you selected can't be >>> used to identify that card1 is the optimized version of card0. >>> >> >> Is userspace currently able to differentiate between cards that are >> created by USB SND versus ASoC? How complex can the userspace card >> discovery be? Can it query kcontrols at this point in time? If so, >> maybe we can change the names of the newly added ones to reflect that it >> is an offload device? >> >> SND kcontrol names are currently: >> Q6USB offload status >> Q6USB offload SND device select > > I must admit I've never seen kcontrols being used to identify what the > card is, and in this case it's a pretend-card that's just an improved > version of another. It might be easier to use something else, such as > the component strings. Its not exactly a pretend card, right? This is part of the overall platform sound card we have in the system. At the moment, I'm only testing by adding the USB audio routing, but there can be several ASoC links defined in the overall platform card. The Q6AFE CPU DAI has multiple audio AFE "ports" they can handle. USB is only one of those. >> >>> Before we review low-level kernel plumbing, it would be good to give a >>> better overview of how userspace applications are supposed to interact >>> with the cards and identify the offloaded path. Testing with >>> tinyplay/tinymix is fine, but that's a developer-level or CI unit test. >>> we've got to see the broader picture of how a sound server would use >>> this USB offload capability. >> >> Sure, I think that is fine. I was hoping that at least adding some of >> the new kcontrols would help userspace make use of this path in general, >> but we can add more information if required. > > Can I ask if this solution has been used with a complete userspace stack > already? I could see how this might be used with a relatively fixed Its been used only with the Android HAL. > Android HAL, where the platform and routing are relatively controlled. I > don't see how a more generic audio server would deal with the discovery > and routing. > This is why your input is helpful, since it provides another use case that wasn't considered. I think in our previous discussions the tagging possibility was a good idea, and was hoping that it could help. Could tag all USB SND cards to the platform sound card as well, and if the power saving path is chosen, it would issue the playback on the platform sound card. (if not in use) In this case, the offload path wouldn't be the default routing, and only enabled for power optimized path. Thanks Wesley Cheng
On 3/13/23 18:43, Wesley Cheng wrote: > Hi Pierre, > > On 3/9/2023 4:37 PM, Pierre-Louis Bossart wrote: >> >>>>> Create vendor ops for the USB SND driver: >>>>> qc_audio_offload: This particular driver has several components >>>>> associated >>>>> with it: >>>>> - QMI stream request handler >>>>> - XHCI interrupter and resource management >>>>> - audio DSP memory management >>>> >>>> so how does this 'qc_audio_offload' interface with 'q6usb' described >>>> above? how are the roles different or complementary? >>>> >>> So in general you can think that the qc_audio_offload is a complement to >>> the USB SND USB class driver, while q6usb is to ASoC. Since the ASoC >> >> Humm, that is far from clear. I don't get how a something that interacts >> with the USB class driver can also be in charge of the audio DSP memory >> management. >> > > This is because the USB class driver is the entity which is going to > work with the USB HCD (XHCI) in this case to fetch the required > addresses, and map that into memory accessible by the audio DSP. It > would be odd to be doing that from the q6usb end, which is part of the > ASoC layer. > >>> framework doesn't have any communication with USB SND, the ASoC DPCM USB >>> backend (q6usb) will have to be the entity that maintains what is going >>> on in USB SND. That way, sessions initiated through the ASoC managed >>> sound card can evaluate what is available based on information reported >>> by q6usb. >>> >>> qc_audio_offload and q6usb will have some interaction between each >>> other. The majority of communication between qc_audio_offload and q6usb >>> is reporting the device connection events. >> >> It's already complicated to figure out how the DSP and USB class driver >> might interact and probe/timing dependencies, but with two additional >> drivers in the mix it's really hard to understand. >> > > I did test some cases based on existence of both these drivers > (qc_audio_offload and q6usb). If either one doesn't exist in the > system, then the offload path would not work. I did improve some of > these potential sequences in the latest revision, such as patch#28. This > would address scenarios where the q6usb ASoC DPCM backend wasn't probed, > while the USB SND (and qc_audio_offload) were still detecting device > connections. > > Once the Q6USB driver is probed, then the offload snd kcontrols would be > created, and devices would be properly identified with the rediscover api. > >> Maybe ascii-art would help describe the concepts and types of >> information exchanged. Maintaining a consistent state across multiple >> drivers is not an easy task. >> > > Hopefully this might help? I know its a lot to read through. It's very helpful! Thanks for spending the time to illustrate the different building blocks. > > USB | ASoC > -------------------------------------------------------------------- > | _________________________ > | |sm8250 platform card | > | |_________________________| > | | | > | ___V____ ____V____ > | |Q6USB | |Q6AFE | #5 > | |"codec" | |"cpu" | > | |________| |_________| > | ^ > | | #6 > | ___V____ > | |SOC-USB | > ________ #1 ________ #7 | | > |USB SND |<--->|QC offld|<------------>|________| > |(card.c)| | |<---------- ^ > |________| |________|___ #4 | | | > ^ ^ | | | ___V__________________ > | #2 | #2 | | | |APR/GLINK | > __ V_______________V_____ | | | |______________________| > |USB SND (endpoint.c) | | | | ^ > |_________________________| | | | #8 | > ^ | | | ___________V___________ > | #3 | | |->|audio DSP | > ___________V_____________ | | |_______________________| > |XHCI HCD |<- | > |_________________________| | > > > #1 - USB SND and QC offload: > Initialization: > - Register platform operations, to receive connect/disconnect events > from USB SND. > - QC offload creates a QMI handle, in order to receive QMI requests > from the audio DSP. > > Runtime: > - USB SND passes along "struct snd_usb_audio" in order for QC offload > to reference USB UAC desc parsing/USB SND helper APIs. > - USB device disconnection events will result in clearing of the chip > entry. > > #2 - USB SND and QC offload endpoints: > Runtime: > - In the non-offloaded path, USB snd will utilize functions exposed by > USB SND endpoint, to help with fetching USB EP references and queuing > URBs. > - In the offload path, qc offload will utilize the functions to fetch > USB EP references, so that it can use that information to query the > XHCI HCD. > - Likewise, both will clean up endpoints when audio stream is not in use. > > #3 - XHCI HCD: > Initialization: > - During XHCI probe timing, when the USB HCD is added to the system, it > will also initialize the secondary event rings. > > Runtime: > - During USB device plug ins/outs, allocates device slot, assigns eps, > and initializes transfer rings. > > #4 - QC offload and XHCI: > Runtime: > - QC offload needs to reference the transfer ring and secondary event ring > addresses by executing XHCI offload management APIs. > - This happens when audio DSP receives a USB QMI stream request. > > #5 - ASoC components: > Initialization: > - The SM8250 platform sound card driver fetches DT node entries defining > the ASoC links. This chain/link has the components involved for a > particular Q6AFE path. (not only USB offload) > - "cpu" - this is the ASoC CPU DAI that handles interaction with the > Q6 DSP's audio protocol. (AFE ports) > - "codec" - the ASoC codec (backend) DAI defined > - Registers ASoC platform sound card based on links defined in the DT node. > - Probes DAI components involved, ie Q6USB and Q6AFE > > Runtime: > - Q6AFE has the bulk of the interaction w/ the audio DSP to start an audio > session, such as issuing AFE port start commands (part of the protocol > used to communicate the audio session info) > - Q6USB will be there to now check for if format requested is supported by > the device, and maintain offloading status. > > #6 - Q6USB and SOC-USB: > Initialization: > - Q6USB will query QC offload for USB device connection states. (through > soc-usb) > - Creates a SOC USB entry, that carries information about resources, > such as audio DSP memory information and requested XHCI event ring > index. > > Runtime: > - SOC-USB will receive connect/disconnect events and propagate to Q6USB. > - Q6USB makes devices available for offloading based on these events. > - Sets Q6AFE port configurations to select the USB SND card# and PCM#. > > #7 - SOC-USB and QC offload: > Initialization: > - Rediscover USB SND devices when the SOC-USB entry is created (if needed) > - For situations where the Q6USB DAI hasn't been probed. > > Runtime: > - Propagate connect/disconnect events. Is the SOC-USB module or building blocks intended to be generic or Qualcomm agnostic? It's not clear to me how it would handle "audio DSP memory information and requested XHCI event ring index." In addition, it seems to be the "bridge" or means of communication between qc_audio_offload and q6usb, is this not based on custom events or triggers? Along the same lines, this SOC-USB entity interfaces with APR/GLINK which doesn't speak to me so it must be a QCOM interface? I am trying to see if this design could be used for other architectures, and the QCOM-specific and generic parts are not obvious. > #8 - audio DSP and QC offload: > Runtime: > - Handle QMI requests coming from audio DSP. These requests come AFTER > the Q6AFE port is opened by the Q6AFE DAI(#6) > - Returns memory information about resources allocated by XHCI. > - Enables audio playback when this QMI transaction is completed. > >>> >>>>> When the audio DSP wants to enable a playback stream, the request is >>>>> first >>>>> received by the ASoC platform sound card. Depending on the selected >>>>> route, >>>>> ASoC will bring up the individual DAIs in the path. The Q6USB >>>>> backend DAI >>>>> will send an AFE port start command (with enabling the USB playback >>>>> path), and >>>>> the audio DSP will handle the request accordingly. >>>>> >>>>> Part of the AFE USB port start handling will have an exchange of >>>>> control >>>>> messages using the QMI protocol. The qc_audio_offload driver will >>>>> populate the >>>>> buffer information: >>>>> - Event ring base address >>>>> - EP transfer ring base address >>>>> >>>>> and pass it along to the audio DSP. All endpoint management will now >>>>> be handed >>>>> over to the DSP, and the main processor is not involved in transfers. >>>>> >>>>> Overall, implementing this feature will still expose separate sound >>>>> card and PCM >>>>> devices for both the platorm card and USB audio device: >>>>> 0 [SM8250MTPWCD938]: sm8250 - SM8250-MTP-WCD9380-WSA8810-VA-D >>>>> SM8250-MTP-WCD9380-WSA8810-VA-DMIC >>>>> 1 [Audio ]: USB-Audio - USB Audio >>>>> Generic USB Audio at usb-xhci-hcd.1.auto-1.4, >>>>> high speed >>>>> >>>>> This is to ensure that userspace ALSA entities can decide which route >>>>> to take >>>>> when executing the audio playback. In the above, if card#1 is >>>>> selected, then >>>>> USB audio data will take the legacy path over the USB PCM drivers, >>>>> etc... >>>> >>>> I already voiced my concerns about exposing two cards, each with their >>>> own set of volume controls with the same device. It would be much >>>> better >>>> to have an additional offloaded PCM device for card0... >>>> >>>> But if the consensus is to have two cards, it's still not clear how the >>>> routing would be selected. In the case where there are two USB audio >>>> devices attached, the offloaded path would only support one of the two. >>>> How would userspace know which of the two is selected? >>>> >>> >>> With patch#24: >>> https://lore.kernel.org/linux-usb/20230308235751.495-25-quic_wcheng@quicinc.com/T/#u >>> >>> Now, userspace can at least choose which device it wants to offload. >>> Part of doing that would mean userspace knows what USB SND card devices >>> are available, so it is aware of which devices are shared (between the >>> offload and USB SND path) >>> >>>> And how would userspace know the difference anyways between two >>>> physical >>>> devices attached to the platform with no offload, and one physical >>>> device with one additional offload path? The names you selected >>>> can't be >>>> used to identify that card1 is the optimized version of card0. >>>> >>> >>> Is userspace currently able to differentiate between cards that are >>> created by USB SND versus ASoC? How complex can the userspace card >>> discovery be? Can it query kcontrols at this point in time? If so, >>> maybe we can change the names of the newly added ones to reflect that it >>> is an offload device? >>> >>> SND kcontrol names are currently: >>> Q6USB offload status >>> Q6USB offload SND device select >> >> I must admit I've never seen kcontrols being used to identify what the >> card is, and in this case it's a pretend-card that's just an improved >> version of another. It might be easier to use something else, such as >> the component strings. > > Its not exactly a pretend card, right? This is part of the overall > platform sound card we have in the system. At the moment, I'm only > testing by adding the USB audio routing, but there can be several ASoC > links defined in the overall platform card. Sorry, I misunderstood the proposal. I thought there would be one card for "generic USB Audio", and another one for "DSP-offloaded USB Audio". I assumed, probably mistakenly, that all local audio endpoints (speaker,mics) would be exposed as a separate card. It looks like it's more "generic USB Audio" and "DSP Audio", with the USB offload being exposed as a PCM device of the latter. Did I get this right? In this case, presumably there can be some sort of UCM file for the "DSP Audio card" that contains the configuration or knows which kcontrols to look for. But my point about detection hold. You could perfectly well have a 'Jack control' that tells userspace when a device is connected. That way there's no guess work, it's similar to HDMI for Intel: the device is exposed but only valid when the jack control is set. > The Q6AFE CPU DAI has multiple audio AFE "ports" they can handle. USB > is only one of those. > >>> >>>> Before we review low-level kernel plumbing, it would be good to give a >>>> better overview of how userspace applications are supposed to interact >>>> with the cards and identify the offloaded path. Testing with >>>> tinyplay/tinymix is fine, but that's a developer-level or CI unit test. >>>> we've got to see the broader picture of how a sound server would use >>>> this USB offload capability. >>> >>> Sure, I think that is fine. I was hoping that at least adding some of >>> the new kcontrols would help userspace make use of this path in general, >>> but we can add more information if required. >> >> Can I ask if this solution has been used with a complete userspace stack >> already? I could see how this might be used with a relatively fixed > > Its been used only with the Android HAL. ok, that aligns with what I was thinking. > >> Android HAL, where the platform and routing are relatively controlled. I >> don't see how a more generic audio server would deal with the discovery >> and routing. >> > > This is why your input is helpful, since it provides another use case > that wasn't considered. I think in our previous discussions the tagging > possibility was a good idea, and was hoping that it could help. Could > tag all USB SND cards to the platform sound card as well, and if the > power saving path is chosen, it would issue the playback on the platform > sound card. (if not in use) > > In this case, the offload path wouldn't be the default routing, and only > enabled for power optimized path. > > Thanks > Wesley Cheng
Hi Pierre, On 3/13/2023 5:42 PM, Pierre-Louis Bossart wrote: > > > On 3/13/23 18:43, Wesley Cheng wrote: >> Hi Pierre, >> >> On 3/9/2023 4:37 PM, Pierre-Louis Bossart wrote: >>> >>>>>> Create vendor ops for the USB SND driver: >>>>>> qc_audio_offload: This particular driver has several components >>>>>> associated >>>>>> with it: >>>>>> - QMI stream request handler >>>>>> - XHCI interrupter and resource management >>>>>> - audio DSP memory management >>>>> >>>>> so how does this 'qc_audio_offload' interface with 'q6usb' described >>>>> above? how are the roles different or complementary? >>>>> >>>> So in general you can think that the qc_audio_offload is a complement to >>>> the USB SND USB class driver, while q6usb is to ASoC. Since the ASoC >>> >>> Humm, that is far from clear. I don't get how a something that interacts >>> with the USB class driver can also be in charge of the audio DSP memory >>> management. >>> >> >> This is because the USB class driver is the entity which is going to >> work with the USB HCD (XHCI) in this case to fetch the required >> addresses, and map that into memory accessible by the audio DSP. It >> would be odd to be doing that from the q6usb end, which is part of the >> ASoC layer. >> >>>> framework doesn't have any communication with USB SND, the ASoC DPCM USB >>>> backend (q6usb) will have to be the entity that maintains what is going >>>> on in USB SND. That way, sessions initiated through the ASoC managed >>>> sound card can evaluate what is available based on information reported >>>> by q6usb. >>>> >>>> qc_audio_offload and q6usb will have some interaction between each >>>> other. The majority of communication between qc_audio_offload and q6usb >>>> is reporting the device connection events. >>> >>> It's already complicated to figure out how the DSP and USB class driver >>> might interact and probe/timing dependencies, but with two additional >>> drivers in the mix it's really hard to understand. >>> >> >> I did test some cases based on existence of both these drivers >> (qc_audio_offload and q6usb). If either one doesn't exist in the >> system, then the offload path would not work. I did improve some of >> these potential sequences in the latest revision, such as patch#28. This >> would address scenarios where the q6usb ASoC DPCM backend wasn't probed, >> while the USB SND (and qc_audio_offload) were still detecting device >> connections. >> >> Once the Q6USB driver is probed, then the offload snd kcontrols would be >> created, and devices would be properly identified with the rediscover api. >> >>> Maybe ascii-art would help describe the concepts and types of >>> information exchanged. Maintaining a consistent state across multiple >>> drivers is not an easy task. >>> >> >> Hopefully this might help? I know its a lot to read through. > > It's very helpful! Thanks for spending the time to illustrate the > different building blocks. > >> >> USB | ASoC >> -------------------------------------------------------------------- >> | _________________________ >> | |sm8250 platform card | >> | |_________________________| >> | | | >> | ___V____ ____V____ >> | |Q6USB | |Q6AFE | #5 >> | |"codec" | |"cpu" | >> | |________| |_________| >> | ^ >> | | #6 >> | ___V____ >> | |SOC-USB | >> ________ #1 ________ #7 | | >> |USB SND |<--->|QC offld|<------------>|________| >> |(card.c)| | |<---------- ^ >> |________| |________|___ #4 | | | >> ^ ^ | | | ___V__________________ >> | #2 | #2 | | | |APR/GLINK | >> __ V_______________V_____ | | | |______________________| >> |USB SND (endpoint.c) | | | | ^ >> |_________________________| | | | #8 | >> ^ | | | ___________V___________ >> | #3 | | |->|audio DSP | >> ___________V_____________ | | |_______________________| >> |XHCI HCD |<- | >> |_________________________| | >> >> >> #1 - USB SND and QC offload: >> Initialization: >> - Register platform operations, to receive connect/disconnect events >> from USB SND. >> - QC offload creates a QMI handle, in order to receive QMI requests >> from the audio DSP. >> >> Runtime: >> - USB SND passes along "struct snd_usb_audio" in order for QC offload >> to reference USB UAC desc parsing/USB SND helper APIs. >> - USB device disconnection events will result in clearing of the chip >> entry. >> >> #2 - USB SND and QC offload endpoints: >> Runtime: >> - In the non-offloaded path, USB snd will utilize functions exposed by >> USB SND endpoint, to help with fetching USB EP references and queuing >> URBs. >> - In the offload path, qc offload will utilize the functions to fetch >> USB EP references, so that it can use that information to query the >> XHCI HCD. >> - Likewise, both will clean up endpoints when audio stream is not in use. >> >> #3 - XHCI HCD: >> Initialization: >> - During XHCI probe timing, when the USB HCD is added to the system, it >> will also initialize the secondary event rings. >> >> Runtime: >> - During USB device plug ins/outs, allocates device slot, assigns eps, >> and initializes transfer rings. >> >> #4 - QC offload and XHCI: >> Runtime: >> - QC offload needs to reference the transfer ring and secondary event ring >> addresses by executing XHCI offload management APIs. >> - This happens when audio DSP receives a USB QMI stream request. >> >> #5 - ASoC components: >> Initialization: >> - The SM8250 platform sound card driver fetches DT node entries defining >> the ASoC links. This chain/link has the components involved for a >> particular Q6AFE path. (not only USB offload) >> - "cpu" - this is the ASoC CPU DAI that handles interaction with the >> Q6 DSP's audio protocol. (AFE ports) >> - "codec" - the ASoC codec (backend) DAI defined >> - Registers ASoC platform sound card based on links defined in the DT node. >> - Probes DAI components involved, ie Q6USB and Q6AFE >> >> Runtime: >> - Q6AFE has the bulk of the interaction w/ the audio DSP to start an audio >> session, such as issuing AFE port start commands (part of the protocol >> used to communicate the audio session info) >> - Q6USB will be there to now check for if format requested is supported by >> the device, and maintain offloading status. >> >> #6 - Q6USB and SOC-USB: >> Initialization: >> - Q6USB will query QC offload for USB device connection states. (through >> soc-usb) >> - Creates a SOC USB entry, that carries information about resources, >> such as audio DSP memory information and requested XHCI event ring >> index. >> >> Runtime: >> - SOC-USB will receive connect/disconnect events and propagate to Q6USB. >> - Q6USB makes devices available for offloading based on these events. >> - Sets Q6AFE port configurations to select the USB SND card# and PCM#. >> >> #7 - SOC-USB and QC offload: >> Initialization: >> - Rediscover USB SND devices when the SOC-USB entry is created (if needed) >> - For situations where the Q6USB DAI hasn't been probed. >> >> Runtime: >> - Propagate connect/disconnect events. > > Is the SOC-USB module or building blocks intended to be generic or > Qualcomm agnostic? > This should be generic. > It's not clear to me how it would handle "audio DSP memory information > and requested XHCI event ring index." > Each soc-usb entry that is created by the ASoC DPCM backend DAI (q6usb) will be able to hold "private data" that, in QC case, is defined as: struct q6usb_offload This is passed within the snd_soc_usb_add_port() call: snd_soc_usb_add_port(component->dev, &data->priv, q6usb_alsa_connection_cb); So depending on the user, the private data can contain their own struct with the information they require. > In addition, it seems to be the "bridge" or means of communication > between qc_audio_offload and q6usb, is this not based on custom events > or triggers? > Ideally, no, it shouldn't be based on custom events. Intention for the connect_cb() that is defined is just to receive USB device discovery events from USB SND. From the qc_audio_offload, we call snd_soc_usb_connect() within our platform op that we register to USB SND. //Platform connect_cb() - called from USB SND probe (device connected) static void qc_usb_audio_offload_probe(struct snd_usb_audio *chip) { ... snd_soc_usb_connect(usb_get_usb_backend(udev), chip->card->number, chip->index, chip->pcm_devs); In the QC situation, we used this to build a list of active devices connected. > Along the same lines, this SOC-USB entity interfaces with APR/GLINK > which doesn't speak to me so it must be a QCOM interface? > Sorry for not labeling those in the diagram, but yes, those are QC specific interfaces. You can just think of it as a type of IPC transport. > I am trying to see if this design could be used for other architectures, > and the QCOM-specific and generic parts are not obvious. > >> #8 - audio DSP and QC offload: >> Runtime: >> - Handle QMI requests coming from audio DSP. These requests come AFTER >> the Q6AFE port is opened by the Q6AFE DAI(#6) >> - Returns memory information about resources allocated by XHCI. >> - Enables audio playback when this QMI transaction is completed. >> >>>> >>>>>> When the audio DSP wants to enable a playback stream, the request is >>>>>> first >>>>>> received by the ASoC platform sound card. Depending on the selected >>>>>> route, >>>>>> ASoC will bring up the individual DAIs in the path. The Q6USB >>>>>> backend DAI >>>>>> will send an AFE port start command (with enabling the USB playback >>>>>> path), and >>>>>> the audio DSP will handle the request accordingly. >>>>>> >>>>>> Part of the AFE USB port start handling will have an exchange of >>>>>> control >>>>>> messages using the QMI protocol. The qc_audio_offload driver will >>>>>> populate the >>>>>> buffer information: >>>>>> - Event ring base address >>>>>> - EP transfer ring base address >>>>>> >>>>>> and pass it along to the audio DSP. All endpoint management will now >>>>>> be handed >>>>>> over to the DSP, and the main processor is not involved in transfers. >>>>>> >>>>>> Overall, implementing this feature will still expose separate sound >>>>>> card and PCM >>>>>> devices for both the platorm card and USB audio device: >>>>>> 0 [SM8250MTPWCD938]: sm8250 - SM8250-MTP-WCD9380-WSA8810-VA-D >>>>>> SM8250-MTP-WCD9380-WSA8810-VA-DMIC >>>>>> 1 [Audio ]: USB-Audio - USB Audio >>>>>> Generic USB Audio at usb-xhci-hcd.1.auto-1.4, >>>>>> high speed >>>>>> >>>>>> This is to ensure that userspace ALSA entities can decide which route >>>>>> to take >>>>>> when executing the audio playback. In the above, if card#1 is >>>>>> selected, then >>>>>> USB audio data will take the legacy path over the USB PCM drivers, >>>>>> etc... >>>>> >>>>> I already voiced my concerns about exposing two cards, each with their >>>>> own set of volume controls with the same device. It would be much >>>>> better >>>>> to have an additional offloaded PCM device for card0... >>>>> >>>>> But if the consensus is to have two cards, it's still not clear how the >>>>> routing would be selected. In the case where there are two USB audio >>>>> devices attached, the offloaded path would only support one of the two. >>>>> How would userspace know which of the two is selected? >>>>> >>>> >>>> With patch#24: >>>> https://lore.kernel.org/linux-usb/20230308235751.495-25-quic_wcheng@quicinc.com/T/#u >>>> >>>> Now, userspace can at least choose which device it wants to offload. >>>> Part of doing that would mean userspace knows what USB SND card devices >>>> are available, so it is aware of which devices are shared (between the >>>> offload and USB SND path) >>>> >>>>> And how would userspace know the difference anyways between two >>>>> physical >>>>> devices attached to the platform with no offload, and one physical >>>>> device with one additional offload path? The names you selected >>>>> can't be >>>>> used to identify that card1 is the optimized version of card0. >>>>> >>>> >>>> Is userspace currently able to differentiate between cards that are >>>> created by USB SND versus ASoC? How complex can the userspace card >>>> discovery be? Can it query kcontrols at this point in time? If so, >>>> maybe we can change the names of the newly added ones to reflect that it >>>> is an offload device? >>>> >>>> SND kcontrol names are currently: >>>> Q6USB offload status >>>> Q6USB offload SND device select >>> >>> I must admit I've never seen kcontrols being used to identify what the >>> card is, and in this case it's a pretend-card that's just an improved >>> version of another. It might be easier to use something else, such as >>> the component strings. >> >> Its not exactly a pretend card, right? This is part of the overall >> platform sound card we have in the system. At the moment, I'm only >> testing by adding the USB audio routing, but there can be several ASoC >> links defined in the overall platform card. > > Sorry, I misunderstood the proposal. I thought there would be one card > for "generic USB Audio", and another one for "DSP-offloaded USB Audio". > I assumed, probably mistakenly, that all local audio endpoints > (speaker,mics) would be exposed as a separate card. > Ah got it. No, that isn't the case here. > It looks like it's more "generic USB Audio" and "DSP Audio", with the > USB offload being exposed as a PCM device of the latter. > > Did I get this right? In this case, presumably there can be some sort of Yep that's correct! > UCM file for the "DSP Audio card" that contains the configuration or > knows which kcontrols to look for. But my point about detection hold. > You could perfectly well have a 'Jack control' that tells userspace when > a device is connected. That way there's no guess work, it's similar to > HDMI for Intel: the device is exposed but only valid when the jack > control is set. > Hmm, ok. Let me see if I can switch up some things. Maybe replace the current snd_soc_dapm_enable_pin() calls in the q6usb connection_cb and replace that with a snd jack report. (the snd jack implementation already takes care of updating the pin if needed) Thanks Wesley Cheng >> The Q6AFE CPU DAI has multiple audio AFE "ports" they can handle. USB >> is only one of those. >> >>>> >>>>> Before we review low-level kernel plumbing, it would be good to give a >>>>> better overview of how userspace applications are supposed to interact >>>>> with the cards and identify the offloaded path. Testing with >>>>> tinyplay/tinymix is fine, but that's a developer-level or CI unit test. >>>>> we've got to see the broader picture of how a sound server would use >>>>> this USB offload capability. >>>> >>>> Sure, I think that is fine. I was hoping that at least adding some of >>>> the new kcontrols would help userspace make use of this path in general, >>>> but we can add more information if required. >>> >>> Can I ask if this solution has been used with a complete userspace stack >>> already? I could see how this might be used with a relatively fixed >> >> Its been used only with the Android HAL. > > ok, that aligns with what I was thinking. > >> >>> Android HAL, where the platform and routing are relatively controlled. I >>> don't see how a more generic audio server would deal with the discovery >>> and routing. >>> >> >> This is why your input is helpful, since it provides another use case >> that wasn't considered. I think in our previous discussions the tagging >> possibility was a good idea, and was hoping that it could help. Could >> tag all USB SND cards to the platform sound card as well, and if the >> power saving path is chosen, it would issue the playback on the platform >> sound card. (if not in use) >> >> In this case, the offload path wouldn't be the default routing, and only >> enabled for power optimized path. >> >> Thanks >> Wesley Cheng
<snip> >> >>> >>> USB | ASoC >>> -------------------------------------------------------------------- >>> | _________________________ >>> | |sm8250 platform card | >>> | |_________________________| >>> | | | >>> | ___V____ ____V____ >>> | |Q6USB | |Q6AFE | #5 >>> | |"codec" | |"cpu" | >>> | |________| |_________| >>> | ^ >>> | | #6 >>> | ___V____ >>> | |SOC-USB | >>> ________ #1 ________ #7 | | >>> |USB SND |<--->|QC offld|<------------>|________| >>> |(card.c)| | |<---------- ^ >>> |________| |________|___ #4 | | | >>> ^ ^ | | | ___V__________________ >>> | #2 | #2 | | | |APR/GLINK | >>> __ V_______________V_____ | | | |______________________| >>> |USB SND (endpoint.c) | | | | ^ >>> |_________________________| | | | #8 | >>> ^ | | | ___________V___________ >>> | #3 | | |->|audio DSP | >>> ___________V_____________ | | |_______________________| >>> |XHCI HCD |<- | >>> |_________________________| | >>> >>> >>> #1 - USB SND and QC offload: >>> Initialization: >>> - Register platform operations, to receive connect/disconnect events >>> from USB SND. >>> - QC offload creates a QMI handle, in order to receive QMI requests >>> from the audio DSP. >>> >>> Runtime: >>> - USB SND passes along "struct snd_usb_audio" in order for QC offload >>> to reference USB UAC desc parsing/USB SND helper APIs. >>> - USB device disconnection events will result in clearing of the chip >>> entry. >>> >>> #2 - USB SND and QC offload endpoints: >>> Runtime: >>> - In the non-offloaded path, USB snd will utilize functions exposed by >>> USB SND endpoint, to help with fetching USB EP references and queuing >>> URBs. >>> - In the offload path, qc offload will utilize the functions to fetch >>> USB EP references, so that it can use that information to query the >>> XHCI HCD. >>> - Likewise, both will clean up endpoints when audio stream is not in >>> use. >>> >>> #3 - XHCI HCD: >>> Initialization: >>> - During XHCI probe timing, when the USB HCD is added to the system, it >>> will also initialize the secondary event rings. >>> >>> Runtime: >>> - During USB device plug ins/outs, allocates device slot, assigns eps, >>> and initializes transfer rings. >>> >>> #4 - QC offload and XHCI: >>> Runtime: >>> - QC offload needs to reference the transfer ring and secondary event >>> ring >>> addresses by executing XHCI offload management APIs. >>> - This happens when audio DSP receives a USB QMI stream request. >>> >>> #5 - ASoC components: >>> Initialization: >>> - The SM8250 platform sound card driver fetches DT node entries defining >>> the ASoC links. This chain/link has the components involved for a >>> particular Q6AFE path. (not only USB offload) >>> - "cpu" - this is the ASoC CPU DAI that handles interaction with >>> the >>> Q6 DSP's audio protocol. (AFE ports) >>> - "codec" - the ASoC codec (backend) DAI defined >>> - Registers ASoC platform sound card based on links defined in the DT >>> node. >>> - Probes DAI components involved, ie Q6USB and Q6AFE >>> >>> Runtime: >>> - Q6AFE has the bulk of the interaction w/ the audio DSP to start an >>> audio >>> session, such as issuing AFE port start commands (part of the >>> protocol >>> used to communicate the audio session info) >>> - Q6USB will be there to now check for if format requested is >>> supported by >>> the device, and maintain offloading status. >>> >>> #6 - Q6USB and SOC-USB: >>> Initialization: >>> - Q6USB will query QC offload for USB device connection states. (through >>> soc-usb) >>> - Creates a SOC USB entry, that carries information about resources, >>> such as audio DSP memory information and requested XHCI event ring >>> index. >>> >>> Runtime: >>> - SOC-USB will receive connect/disconnect events and propagate to Q6USB. >>> - Q6USB makes devices available for offloading based on these events. >>> - Sets Q6AFE port configurations to select the USB SND card# and PCM#. >>> >>> #7 - SOC-USB and QC offload: >>> Initialization: >>> - Rediscover USB SND devices when the SOC-USB entry is created (if >>> needed) >>> - For situations where the Q6USB DAI hasn't been probed. >>> >>> Runtime: >>> - Propagate connect/disconnect events. >> >> Is the SOC-USB module or building blocks intended to be generic or >> Qualcomm agnostic? >> > > This should be generic. ok, but then how would it communicate with APR/GLINK described below [1] > >> It's not clear to me how it would handle "audio DSP memory information >> and requested XHCI event ring index." >> > > Each soc-usb entry that is created by the ASoC DPCM backend DAI (q6usb) > will be able to hold "private data" that, in QC case, is defined as: > struct q6usb_offload > > This is passed within the snd_soc_usb_add_port() call: > snd_soc_usb_add_port(component->dev, &data->priv, > q6usb_alsa_connection_cb); > > So depending on the user, the private data can contain their own struct > with the information they require. ok, so "handle private data such as audio DSP memory information >> and requested XHCI event ring index" is what you meant. That'd fine. >> In addition, it seems to be the "bridge" or means of communication >> between qc_audio_offload and q6usb, is this not based on custom events >> or triggers? >> > > Ideally, no, it shouldn't be based on custom events. Intention for the > connect_cb() that is defined is just to receive USB device discovery > events from USB SND. From the qc_audio_offload, we call > snd_soc_usb_connect() within our platform op that we register to USB SND. > > //Platform connect_cb() - called from USB SND probe (device connected) > static void qc_usb_audio_offload_probe(struct snd_usb_audio *chip) > { > ... > snd_soc_usb_connect(usb_get_usb_backend(udev), chip->card->number, > chip->index, chip->pcm_devs); > > In the QC situation, we used this to build a list of active devices > connected. > >> Along the same lines, this SOC-USB entity interfaces with APR/GLINK >> which doesn't speak to me so it must be a QCOM interface?>> > > Sorry for not labeling those in the diagram, but yes, those are QC > specific interfaces. You can just think of it as a type of IPC transport. [1] ... something's not clear on how a generic 'soc-usb' component can directly talk to a vendor-specific IPC. Is there a missing layer? >> I am trying to see if this design could be used for other architectures, >> and the QCOM-specific and generic parts are not obvious. >> >>> #8 - audio DSP and QC offload: >>> Runtime: >>> - Handle QMI requests coming from audio DSP. These requests come AFTER >>> the Q6AFE port is opened by the Q6AFE DAI(#6) >>> - Returns memory information about resources allocated by XHCI. >>> - Enables audio playback when this QMI transaction is completed. >>> >>>>> >>>>>>> When the audio DSP wants to enable a playback stream, the request is >>>>>>> first >>>>>>> received by the ASoC platform sound card. Depending on the selected >>>>>>> route, >>>>>>> ASoC will bring up the individual DAIs in the path. The Q6USB >>>>>>> backend DAI >>>>>>> will send an AFE port start command (with enabling the USB playback >>>>>>> path), and >>>>>>> the audio DSP will handle the request accordingly. >>>>>>> >>>>>>> Part of the AFE USB port start handling will have an exchange of >>>>>>> control >>>>>>> messages using the QMI protocol. The qc_audio_offload driver will >>>>>>> populate the >>>>>>> buffer information: >>>>>>> - Event ring base address >>>>>>> - EP transfer ring base address >>>>>>> >>>>>>> and pass it along to the audio DSP. All endpoint management will >>>>>>> now >>>>>>> be handed >>>>>>> over to the DSP, and the main processor is not involved in >>>>>>> transfers. >>>>>>> >>>>>>> Overall, implementing this feature will still expose separate sound >>>>>>> card and PCM >>>>>>> devices for both the platorm card and USB audio device: >>>>>>> 0 [SM8250MTPWCD938]: sm8250 - SM8250-MTP-WCD9380-WSA8810-VA-D >>>>>>> SM8250-MTP-WCD9380-WSA8810-VA-DMIC >>>>>>> 1 [Audio ]: USB-Audio - USB Audio >>>>>>> Generic USB Audio at >>>>>>> usb-xhci-hcd.1.auto-1.4, >>>>>>> high speed >>>>>>> >>>>>>> This is to ensure that userspace ALSA entities can decide which >>>>>>> route >>>>>>> to take >>>>>>> when executing the audio playback. In the above, if card#1 is >>>>>>> selected, then >>>>>>> USB audio data will take the legacy path over the USB PCM drivers, >>>>>>> etc... >>>>>> >>>>>> I already voiced my concerns about exposing two cards, each with >>>>>> their >>>>>> own set of volume controls with the same device. It would be much >>>>>> better >>>>>> to have an additional offloaded PCM device for card0... >>>>>> >>>>>> But if the consensus is to have two cards, it's still not clear >>>>>> how the >>>>>> routing would be selected. In the case where there are two USB audio >>>>>> devices attached, the offloaded path would only support one of the >>>>>> two. >>>>>> How would userspace know which of the two is selected? >>>>>> >>>>> >>>>> With patch#24: >>>>> https://lore.kernel.org/linux-usb/20230308235751.495-25-quic_wcheng@quicinc.com/T/#u >>>>> >>>>> Now, userspace can at least choose which device it wants to offload. >>>>> Part of doing that would mean userspace knows what USB SND card >>>>> devices >>>>> are available, so it is aware of which devices are shared (between the >>>>> offload and USB SND path) >>>>> >>>>>> And how would userspace know the difference anyways between two >>>>>> physical >>>>>> devices attached to the platform with no offload, and one physical >>>>>> device with one additional offload path? The names you selected >>>>>> can't be >>>>>> used to identify that card1 is the optimized version of card0. >>>>>> >>>>> >>>>> Is userspace currently able to differentiate between cards that are >>>>> created by USB SND versus ASoC? How complex can the userspace card >>>>> discovery be? Can it query kcontrols at this point in time? If so, >>>>> maybe we can change the names of the newly added ones to reflect >>>>> that it >>>>> is an offload device? >>>>> >>>>> SND kcontrol names are currently: >>>>> Q6USB offload status >>>>> Q6USB offload SND device select >>>> >>>> I must admit I've never seen kcontrols being used to identify what the >>>> card is, and in this case it's a pretend-card that's just an improved >>>> version of another. It might be easier to use something else, such as >>>> the component strings. >>> >>> Its not exactly a pretend card, right? This is part of the overall >>> platform sound card we have in the system. At the moment, I'm only >>> testing by adding the USB audio routing, but there can be several ASoC >>> links defined in the overall platform card. >> >> Sorry, I misunderstood the proposal. I thought there would be one card >> for "generic USB Audio", and another one for "DSP-offloaded USB Audio". >> I assumed, probably mistakenly, that all local audio endpoints >> (speaker,mics) would be exposed as a separate card. >> > > Ah got it. No, that isn't the case here. > >> It looks like it's more "generic USB Audio" and "DSP Audio", with the >> USB offload being exposed as a PCM device of the latter. >> >> Did I get this right? In this case, presumably there can be some sort of > > Yep that's correct! ok, that's good. My initial thought was to add a 'DSP offload' PCM to the USB card, you added a "USB offload" PCM to the DSP card. Nice logical swap! Your proposal might be easier in practice since there's typically a vendor-specific configuration file (UCM or custom) file for the DSP, where USB information can be added. It's more problematic to change a generic USB card as we know it today and bolt vendor-specific DSP information on top. The only open I have with your option is that there are still two control paths to e.g. set the volume. It would be so much easier for userspace if there was a single volume control no matter what path is used for data, or make sure the kcontrols are 'mirrored' somehow. If we found a way to address this issue that would be ideal. >> UCM file for the "DSP Audio card" that contains the configuration or >> knows which kcontrols to look for. But my point about detection hold. >> You could perfectly well have a 'Jack control' that tells userspace when >> a device is connected. That way there's no guess work, it's similar to >> HDMI for Intel: the device is exposed but only valid when the jack >> control is set. >> > > Hmm, ok. Let me see if I can switch up some things. Maybe replace the > current snd_soc_dapm_enable_pin() calls in the q6usb connection_cb and > replace that with a snd jack report. (the snd jack implementation > already takes care of updating the pin if needed) The jack is useful to let userspace know if a PCM device, i.e. a Front-End, can be used. But if you expose a PCM device, nothing prevents an application from trying to open and use it, we recently had such an issue due to a change in PipeWire that tried to open a non-functional HDMI device. So you do need something to bail if the PCM device is mistakenly used. DAPM pin management seems different, it will turn-on/off parts of the graph connected to an endpoint. Userpace will typically not know anything about pin management, it's an in-kernel concept. Not sure if you have to choose, those are two different layers, no?
Hi Pierre, On 3/13/2023 7:22 PM, Pierre-Louis Bossart wrote: > > <snip> > >>> >>>> >>>> USB | ASoC >>>> -------------------------------------------------------------------- >>>> | _________________________ >>>> | |sm8250 platform card | >>>> | |_________________________| >>>> | | | >>>> | ___V____ ____V____ >>>> | |Q6USB | |Q6AFE | #5 >>>> | |"codec" | |"cpu" | >>>> | |________| |_________| >>>> | ^ >>>> | | #6 >>>> | ___V____ >>>> | |SOC-USB | >>>> ________ #1 ________ #7 | | >>>> |USB SND |<--->|QC offld|<------------>|________| >>>> |(card.c)| | |<---------- ^ >>>> |________| |________|___ #4 | | | >>>> ^ ^ | | | ___V__________________ >>>> | #2 | #2 | | | |APR/GLINK | >>>> __ V_______________V_____ | | | |______________________| >>>> |USB SND (endpoint.c) | | | | ^ >>>> |_________________________| | | | #8 | >>>> ^ | | | ___________V___________ >>>> | #3 | | |->|audio DSP | >>>> ___________V_____________ | | |_______________________| >>>> |XHCI HCD |<- | >>>> |_________________________| | >>>> >>>> >>>> #1 - USB SND and QC offload: >>>> Initialization: >>>> - Register platform operations, to receive connect/disconnect events >>>> from USB SND. >>>> - QC offload creates a QMI handle, in order to receive QMI requests >>>> from the audio DSP. >>>> >>>> Runtime: >>>> - USB SND passes along "struct snd_usb_audio" in order for QC offload >>>> to reference USB UAC desc parsing/USB SND helper APIs. >>>> - USB device disconnection events will result in clearing of the chip >>>> entry. >>>> >>>> #2 - USB SND and QC offload endpoints: >>>> Runtime: >>>> - In the non-offloaded path, USB snd will utilize functions exposed by >>>> USB SND endpoint, to help with fetching USB EP references and queuing >>>> URBs. >>>> - In the offload path, qc offload will utilize the functions to fetch >>>> USB EP references, so that it can use that information to query the >>>> XHCI HCD. >>>> - Likewise, both will clean up endpoints when audio stream is not in >>>> use. >>>> >>>> #3 - XHCI HCD: >>>> Initialization: >>>> - During XHCI probe timing, when the USB HCD is added to the system, it >>>> will also initialize the secondary event rings. >>>> >>>> Runtime: >>>> - During USB device plug ins/outs, allocates device slot, assigns eps, >>>> and initializes transfer rings. >>>> >>>> #4 - QC offload and XHCI: >>>> Runtime: >>>> - QC offload needs to reference the transfer ring and secondary event >>>> ring >>>> addresses by executing XHCI offload management APIs. >>>> - This happens when audio DSP receives a USB QMI stream request. >>>> >>>> #5 - ASoC components: >>>> Initialization: >>>> - The SM8250 platform sound card driver fetches DT node entries defining >>>> the ASoC links. This chain/link has the components involved for a >>>> particular Q6AFE path. (not only USB offload) >>>> - "cpu" - this is the ASoC CPU DAI that handles interaction with >>>> the >>>> Q6 DSP's audio protocol. (AFE ports) >>>> - "codec" - the ASoC codec (backend) DAI defined >>>> - Registers ASoC platform sound card based on links defined in the DT >>>> node. >>>> - Probes DAI components involved, ie Q6USB and Q6AFE >>>> >>>> Runtime: >>>> - Q6AFE has the bulk of the interaction w/ the audio DSP to start an >>>> audio >>>> session, such as issuing AFE port start commands (part of the >>>> protocol >>>> used to communicate the audio session info) >>>> - Q6USB will be there to now check for if format requested is >>>> supported by >>>> the device, and maintain offloading status. >>>> >>>> #6 - Q6USB and SOC-USB: >>>> Initialization: >>>> - Q6USB will query QC offload for USB device connection states. (through >>>> soc-usb) >>>> - Creates a SOC USB entry, that carries information about resources, >>>> such as audio DSP memory information and requested XHCI event ring >>>> index. >>>> >>>> Runtime: >>>> - SOC-USB will receive connect/disconnect events and propagate to Q6USB. >>>> - Q6USB makes devices available for offloading based on these events. >>>> - Sets Q6AFE port configurations to select the USB SND card# and PCM#. >>>> >>>> #7 - SOC-USB and QC offload: >>>> Initialization: >>>> - Rediscover USB SND devices when the SOC-USB entry is created (if >>>> needed) >>>> - For situations where the Q6USB DAI hasn't been probed. >>>> >>>> Runtime: >>>> - Propagate connect/disconnect events. >>> >>> Is the SOC-USB module or building blocks intended to be generic or >>> Qualcomm agnostic? >>> >> >> This should be generic. > > ok, but then how would it communicate with APR/GLINK described below [1] To make the diagram a bit more compact, I left out our Q6ASM DAI, which is the path that handles the PCM data/buffers. (not really involved in initializing any part of the offload path) Sorry made a mistake on the diagram. There is no connection from SOC-USB to the APR/GLINK. The Q6USB driver will be the one that is going to configure some of the Q6AFE ports along withe the Q6AFE DAI driver. | ASoC ---------------------------------- | _________________________ | |sm8250 platform card | | |_________________________| | | | | ___V____ ____V____ | |Q6USB | |Q6AFE | #5 | |"codec" | |"cpu" | | |________| |_________| | ^ ^ ^ | #6 | |________| | ___V____ | | |SOC-USB | | #7 | | | ----->|________| | --- | | | | | | _____________V________ | | |APR/GLINK | | | |______________________| | | ^ | | #8 | | | ___________V___________ | |->|audio DSP | | |_______________________| | | >> >>> It's not clear to me how it would handle "audio DSP memory information >>> and requested XHCI event ring index." >>> >> >> Each soc-usb entry that is created by the ASoC DPCM backend DAI (q6usb) >> will be able to hold "private data" that, in QC case, is defined as: >> struct q6usb_offload >> >> This is passed within the snd_soc_usb_add_port() call: >> snd_soc_usb_add_port(component->dev, &data->priv, >> q6usb_alsa_connection_cb); >> >> So depending on the user, the private data can contain their own struct >> with the information they require. > > ok, so "handle private data such as audio DSP memory information >>> and requested XHCI event ring index" is what you meant. That'd fine. > >>> In addition, it seems to be the "bridge" or means of communication >>> between qc_audio_offload and q6usb, is this not based on custom events >>> or triggers? >>> >> >> Ideally, no, it shouldn't be based on custom events. Intention for the >> connect_cb() that is defined is just to receive USB device discovery >> events from USB SND. From the qc_audio_offload, we call >> snd_soc_usb_connect() within our platform op that we register to USB SND. >> >> //Platform connect_cb() - called from USB SND probe (device connected) >> static void qc_usb_audio_offload_probe(struct snd_usb_audio *chip) >> { >> ... >> snd_soc_usb_connect(usb_get_usb_backend(udev), chip->card->number, >> chip->index, chip->pcm_devs); >> >> In the QC situation, we used this to build a list of active devices >> connected. >> >>> Along the same lines, this SOC-USB entity interfaces with APR/GLINK >>> which doesn't speak to me so it must be a QCOM interface?>> >> >> Sorry for not labeling those in the diagram, but yes, those are QC >> specific interfaces. You can just think of it as a type of IPC transport. > > [1] ... something's not clear on how a generic 'soc-usb' component can > directly talk to a vendor-specific IPC. > > Is there a missing layer? > :) hopefully the updated diagram clarifies this. >>> I am trying to see if this design could be used for other architectures, >>> and the QCOM-specific and generic parts are not obvious. >>> >>>> #8 - audio DSP and QC offload: >>>> Runtime: >>>> - Handle QMI requests coming from audio DSP. These requests come AFTER >>>> the Q6AFE port is opened by the Q6AFE DAI(#6) >>>> - Returns memory information about resources allocated by XHCI. >>>> - Enables audio playback when this QMI transaction is completed. >>>> >>>>>> >>>>>>>> When the audio DSP wants to enable a playback stream, the request is >>>>>>>> first >>>>>>>> received by the ASoC platform sound card. Depending on the selected >>>>>>>> route, >>>>>>>> ASoC will bring up the individual DAIs in the path. The Q6USB >>>>>>>> backend DAI >>>>>>>> will send an AFE port start command (with enabling the USB playback >>>>>>>> path), and >>>>>>>> the audio DSP will handle the request accordingly. >>>>>>>> >>>>>>>> Part of the AFE USB port start handling will have an exchange of >>>>>>>> control >>>>>>>> messages using the QMI protocol. The qc_audio_offload driver will >>>>>>>> populate the >>>>>>>> buffer information: >>>>>>>> - Event ring base address >>>>>>>> - EP transfer ring base address >>>>>>>> >>>>>>>> and pass it along to the audio DSP. All endpoint management will >>>>>>>> now >>>>>>>> be handed >>>>>>>> over to the DSP, and the main processor is not involved in >>>>>>>> transfers. >>>>>>>> >>>>>>>> Overall, implementing this feature will still expose separate sound >>>>>>>> card and PCM >>>>>>>> devices for both the platorm card and USB audio device: >>>>>>>> 0 [SM8250MTPWCD938]: sm8250 - SM8250-MTP-WCD9380-WSA8810-VA-D >>>>>>>> SM8250-MTP-WCD9380-WSA8810-VA-DMIC >>>>>>>> 1 [Audio ]: USB-Audio - USB Audio >>>>>>>> Generic USB Audio at >>>>>>>> usb-xhci-hcd.1.auto-1.4, >>>>>>>> high speed >>>>>>>> >>>>>>>> This is to ensure that userspace ALSA entities can decide which >>>>>>>> route >>>>>>>> to take >>>>>>>> when executing the audio playback. In the above, if card#1 is >>>>>>>> selected, then >>>>>>>> USB audio data will take the legacy path over the USB PCM drivers, >>>>>>>> etc... >>>>>>> >>>>>>> I already voiced my concerns about exposing two cards, each with >>>>>>> their >>>>>>> own set of volume controls with the same device. It would be much >>>>>>> better >>>>>>> to have an additional offloaded PCM device for card0... >>>>>>> >>>>>>> But if the consensus is to have two cards, it's still not clear >>>>>>> how the >>>>>>> routing would be selected. In the case where there are two USB audio >>>>>>> devices attached, the offloaded path would only support one of the >>>>>>> two. >>>>>>> How would userspace know which of the two is selected? >>>>>>> >>>>>> >>>>>> With patch#24: >>>>>> https://lore.kernel.org/linux-usb/20230308235751.495-25-quic_wcheng@quicinc.com/T/#u >>>>>> >>>>>> Now, userspace can at least choose which device it wants to offload. >>>>>> Part of doing that would mean userspace knows what USB SND card >>>>>> devices >>>>>> are available, so it is aware of which devices are shared (between the >>>>>> offload and USB SND path) >>>>>> >>>>>>> And how would userspace know the difference anyways between two >>>>>>> physical >>>>>>> devices attached to the platform with no offload, and one physical >>>>>>> device with one additional offload path? The names you selected >>>>>>> can't be >>>>>>> used to identify that card1 is the optimized version of card0. >>>>>>> >>>>>> >>>>>> Is userspace currently able to differentiate between cards that are >>>>>> created by USB SND versus ASoC? How complex can the userspace card >>>>>> discovery be? Can it query kcontrols at this point in time? If so, >>>>>> maybe we can change the names of the newly added ones to reflect >>>>>> that it >>>>>> is an offload device? >>>>>> >>>>>> SND kcontrol names are currently: >>>>>> Q6USB offload status >>>>>> Q6USB offload SND device select >>>>> >>>>> I must admit I've never seen kcontrols being used to identify what the >>>>> card is, and in this case it's a pretend-card that's just an improved >>>>> version of another. It might be easier to use something else, such as >>>>> the component strings. >>>> >>>> Its not exactly a pretend card, right? This is part of the overall >>>> platform sound card we have in the system. At the moment, I'm only >>>> testing by adding the USB audio routing, but there can be several ASoC >>>> links defined in the overall platform card. >>> >>> Sorry, I misunderstood the proposal. I thought there would be one card >>> for "generic USB Audio", and another one for "DSP-offloaded USB Audio". >>> I assumed, probably mistakenly, that all local audio endpoints >>> (speaker,mics) would be exposed as a separate card. >>> >> >> Ah got it. No, that isn't the case here. >> >>> It looks like it's more "generic USB Audio" and "DSP Audio", with the >>> USB offload being exposed as a PCM device of the latter. >>> >>> Did I get this right? In this case, presumably there can be some sort of >> >> Yep that's correct! > > ok, that's good. > > My initial thought was to add a 'DSP offload' PCM to the USB card, you > added a "USB offload" PCM to the DSP card. Nice logical swap! > > Your proposal might be easier in practice since there's typically a > vendor-specific configuration file (UCM or custom) file for the DSP, > where USB information can be added. > > It's more problematic to change a generic USB card as we know it today > and bolt vendor-specific DSP information on top. > > The only open I have with your option is that there are still two > control paths to e.g. set the volume. It would be so much easier for > userspace if there was a single volume control no matter what path is > used for data, or make sure the kcontrols are 'mirrored' somehow. If we > found a way to address this issue that would be ideal. > Got it. Let me look to see if that is something we can address/add. I think the current implementation is that USB SND will expose some mixer controls based on the UAC descriptor parsing. Then when they want to change the volume (for example) it will result in a USB SETUP transaction. So USB SND will eventually be the entity controlling these changes. >>> UCM file for the "DSP Audio card" that contains the configuration or >>> knows which kcontrols to look for. But my point about detection hold. >>> You could perfectly well have a 'Jack control' that tells userspace when >>> a device is connected. That way there's no guess work, it's similar to >>> HDMI for Intel: the device is exposed but only valid when the jack >>> control is set. >>> >> >> Hmm, ok. Let me see if I can switch up some things. Maybe replace the >> current snd_soc_dapm_enable_pin() calls in the q6usb connection_cb and >> replace that with a snd jack report. (the snd jack implementation >> already takes care of updating the pin if needed) > > The jack is useful to let userspace know if a PCM device, i.e. a > Front-End, can be used. But if you expose a PCM device, nothing prevents > an application from trying to open and use it, we recently had such an > issue due to a change in PipeWire that tried to open a non-functional > HDMI device. So you do need something to bail if the PCM device is > mistakenly used. > > DAPM pin management seems different, it will turn-on/off parts of the > graph connected to an endpoint. Userpace will typically not know > anything about pin management, it's an in-kernel concept. > > Not sure if you have to choose, those are two different layers, no? Will review this more and fix it in the next rev. Thanks for the inputs! Thanks Wesley Cheng
Hi Wesley, > Sorry made a mistake on the diagram. There is no connection from > SOC-USB to the APR/GLINK. The Q6USB driver will be the one that is > going to configure some of the Q6AFE ports along withe the Q6AFE DAI > driver. > > | ASoC > ---------------------------------- > | _________________________ > | |sm8250 platform card | > | |_________________________| > | | | > | ___V____ ____V____ > | |Q6USB | |Q6AFE | #5 > | |"codec" | |"cpu" | > | |________| |_________| > | ^ ^ ^ > | #6 | |________| > | ___V____ | > | |SOC-USB | | > #7 | | | > ----->|________| | > --- | > | | | > | | _____________V________ > | | |APR/GLINK | > | | |______________________| > | | ^ > | | #8 | > | | ___________V___________ > | |->|audio DSP | > | |_______________________| > | > | > >>> Makes sense now, thank you for the clarification. I'll have to dig more in this 'soc-usb' block, it's clearly a key component that will have to maintain a consistent state across two different parts of the stack and deal with probe/remove/shutdown. >> My initial thought was to add a 'DSP offload' PCM to the USB card, you >> added a "USB offload" PCM to the DSP card. Nice logical swap! >> >> Your proposal might be easier in practice since there's typically a >> vendor-specific configuration file (UCM or custom) file for the DSP, >> where USB information can be added. >> >> It's more problematic to change a generic USB card as we know it today >> and bolt vendor-specific DSP information on top. >> >> The only open I have with your option is that there are still two >> control paths to e.g. set the volume. It would be so much easier for >> userspace if there was a single volume control no matter what path is >> used for data, or make sure the kcontrols are 'mirrored' somehow. If we >> found a way to address this issue that would be ideal. >> > > Got it. Let me look to see if that is something we can address/add. I > think the current implementation is that USB SND will expose some mixer > controls based on the UAC descriptor parsing. Then when they want to > change the volume (for example) it will result in a USB SETUP transaction. > > So USB SND will eventually be the entity controlling these changes. That's probably ok then, am I getting this right that the the DSP card would not expose any USB-related kcontrols then, i.e. the ONLY path to change volumes, etc., would through the regular USB card kcontrols? That would limit the changes in the platform sound card to the addition of a PCM USB device.
On Wed, Mar 15, 2023 at 09:30:58AM -0500, Pierre-Louis Bossart wrote: > That's probably ok then, am I getting this right that the the DSP card > would not expose any USB-related kcontrols then, i.e. the ONLY path to > change volumes, etc., would through the regular USB card kcontrols? > That would limit the changes in the platform sound card to the addition > of a PCM USB device. I'd guess that there might be some volume control in the DSP as part of the generic routing to the port (eg, if all ports have some output control) but I'm not sure that's an issue, could even be useful for normalising the output of DSP algorithms compared to direct PCM playback.
Hi Pierre, On 3/15/2023 7:30 AM, Pierre-Louis Bossart wrote: > Hi Wesley, > >> Sorry made a mistake on the diagram. There is no connection from >> SOC-USB to the APR/GLINK. The Q6USB driver will be the one that is >> going to configure some of the Q6AFE ports along withe the Q6AFE DAI >> driver. >> >> | ASoC >> ---------------------------------- >> | _________________________ >> | |sm8250 platform card | >> | |_________________________| >> | | | >> | ___V____ ____V____ >> | |Q6USB | |Q6AFE | #5 >> | |"codec" | |"cpu" | >> | |________| |_________| >> | ^ ^ ^ >> | #6 | |________| >> | ___V____ | >> | |SOC-USB | | >> #7 | | | >> ----->|________| | >> --- | >> | | | >> | | _____________V________ >> | | |APR/GLINK | >> | | |______________________| >> | | ^ >> | | #8 | >> | | ___________V___________ >> | |->|audio DSP | >> | |_______________________| >> | >> | >> >>>> > > Makes sense now, thank you for the clarification. > > I'll have to dig more in this 'soc-usb' block, it's clearly a key > component that will have to maintain a consistent state across two > different parts of the stack and deal with probe/remove/shutdown. > >>> My initial thought was to add a 'DSP offload' PCM to the USB card, you >>> added a "USB offload" PCM to the DSP card. Nice logical swap! >>> >>> Your proposal might be easier in practice since there's typically a >>> vendor-specific configuration file (UCM or custom) file for the DSP, >>> where USB information can be added. >>> >>> It's more problematic to change a generic USB card as we know it today >>> and bolt vendor-specific DSP information on top. >>> >>> The only open I have with your option is that there are still two >>> control paths to e.g. set the volume. It would be so much easier for >>> userspace if there was a single volume control no matter what path is >>> used for data, or make sure the kcontrols are 'mirrored' somehow. If we >>> found a way to address this issue that would be ideal. >>> >> >> Got it. Let me look to see if that is something we can address/add. I >> think the current implementation is that USB SND will expose some mixer >> controls based on the UAC descriptor parsing. Then when they want to >> change the volume (for example) it will result in a USB SETUP transaction. >> >> So USB SND will eventually be the entity controlling these changes. > > That's probably ok then, am I getting this right that the the DSP card > would not expose any USB-related kcontrols then, i.e. the ONLY path to > change volumes, etc., would through the regular USB card kcontrols? > > That would limit the changes in the platform sound card to the addition > of a PCM USB device. Yes, that's correct. There won't be any exposed USB volume controls from the DSP card. Thanks Wesley Cheng
Hi Srini, On 3/9/2023 11:21 PM, Srinivas Kandagatla wrote: > > > On 09/03/2023 19:38, Wesley Cheng wrote: >> Hi Srinivas, >> >> On 3/9/2023 1:01 AM, Srinivas Kandagatla wrote: >>> >>> >>> On 08/03/2023 23:57, Wesley Cheng wrote: >>>> Create a USB BE component that will register a new USB port to the >>>> ASoC USB >>>> framework. This will handle determination on if the requested audio >>>> profile is supported by the USB device currently selected. >>>> >>>> Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com> >>> >>> Thanks Wesley for the patch, I have few minor comments. >>> >> >> Thanks for the review! >> >>>> --- >>>> include/sound/q6usboffload.h | 20 ++++ >>>> sound/soc/qcom/Kconfig | 4 + >>>> sound/soc/qcom/qdsp6/Makefile | 1 + >>>> sound/soc/qcom/qdsp6/q6usb.c | 208 >>>> ++++++++++++++++++++++++++++++++++ >>>> 4 files changed, 233 insertions(+) >>>> create mode 100644 include/sound/q6usboffload.h >>>> create mode 100644 sound/soc/qcom/qdsp6/q6usb.c >>>> >>>> diff --git a/include/sound/q6usboffload.h >>>> b/include/sound/q6usboffload.h >>>> new file mode 100644 >>>> index 000000000000..4fb1912d9f55 >>>> --- /dev/null >>>> +++ b/include/sound/q6usboffload.h >>>> @@ -0,0 +1,20 @@ >>>> +/* SPDX-License-Identifier: GPL-2.0 >>>> + * >>>> + * linux/sound/q6usboffload.h -- QDSP6 USB offload >>>> + * >>>> + * Copyright (c) 2022-2023 Qualcomm Innovation Center, Inc. All >>>> rights reserved. >>>> + */ >>>> + >>>> +/** >>>> + * struct q6usb_offload >>>> + * @dev - dev handle to usb be >>>> + * @sid - streamID for iommu >>>> + * @intr_num - usb interrupter number >>>> + * @domain - allocated iommu domain >>>> + **/ >>>> +struct q6usb_offload { >>>> + struct device *dev; >>>> + long long sid; >>>> + u32 intr_num; >>>> + struct iommu_domain *domain; >>> Why do we need to store this domain, You can remove this along with >>> the one line that gets domain in probe function. >>> >> >> We'll need a reference to the iommu domain, because the QC USB offload >> driver will be the one that is going to map the XHCI interrupter and >> transfer ring regions for the audio DSP. This happens when a USB QMI > > this is okay, AFAIU, as long as uaudio_qdev->dev pointer is used in dma > alloc apis like dma_map*, dma_alloc_* you would not need to handle > iommu_domain directly like this in drivers. > Was looking into this a bit more, but couldn't figure out the proper DMA API that would achieve what we're attempting to do. So we are trying to map the PA of the memory allocated by the XHCI driver, for say...the xfer buffers being used for the ISOC ep. If I call the DMA MAP variants, ie dma_map_single_attrs, we pass in a virtual address only, and the DMA map api will map to a PA that it derives from that. However, we're trying to map to memory accessible by the audio DSP, which is the virtual address we're passing into iommu_map(), and the PA mapping to is the XHCI xfer buffer. There wasn't a similar API that achieved this. Please clarify if there might be something I'm missing. Thanks Wesley Cheng
Changes in v3: - Changed prefix from RFC to PATCH - Rebased entire series to usb-next - Updated copyright years XHCI: - Rebased changes on top of XHCI changes merged into usb-next, and only added changes that were still under discussion. - Added change to read in the "num-hc-interrupters" device property. ASoC: - qusb6 USB backend - Incorporated suggestions to fetch iommu information with existing APIs - Added two new sound kcontrols to fetch offload status and offload device selection. - offload status - will return the card and pcm device in use tinymix -D 0 get 1 --> 1, 0 (offload in progress on card#1 pcm#0) - device selection - set the card and pcm device to enable offload on. Ex.: tinymix -D 0 set 1 2 0 --> sets offload on card#2 pcm#0 (this should be the USB card) USB SND: - Fixed up some locking related concerns for registering platform ops. - Moved callbacks under the register_mutex, so that - Modified APIs to properly pass more information about the USB SND device, so that the Q6USB backend can build a device list/map, in order to monitor offload status and device selection. Changes in v2: XHCI: - Replaced XHCI and HCD changes with Mathias' XHCI interrupter changes in his tree: https://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git/log/?h=feature_interrupters Adjustments made to Mathias' changes: - Created xhci-intr.h to export/expose interrupter APIs versus exposing xhci.h. Moved dependent structures to this file as well. (so clients can parse out information from "struct xhci_interrupter") - Added some basic locking when requesting interrupters. - Fixed up some sanity checks. - Removed clearing of the ERSTBA during freeing of the interrupter. (pending issue where SMMU fault occurs if DMA addr returned is 64b - TODO) - Clean up pending events in the XHCI secondary interrupter. While testing USB bus suspend, it was seen that on bus resume, the xHCI HC would run into a command timeout. - Added offloading APIs to xHCI to fetch transfer and event ring information. ASoC: - Modified soc-usb to allow for multiple USB port additions. For this to work, the USB offload driver has to have a reference to the USB backend by adding a "usb-soc-be" DT entry to the device saved into XHCI sysdev. - Created separate dt-bindings for defining USB_RX port. - Increased APR timeout to accommodate the situation where the AFE port start command could be delayed due to having to issue a USB bus resume while handling the QMI stream start command. USB SND: - Added a platform ops during usb_audio_suspend(). This allows for the USB offload driver to halt the audio stream when system enters PM suspend. This ensures the audio DSP is not issuing transfers on the USB bus. - Do not override platform ops if they are already populated. - Introduce a shared status variable between the USB offload and USB SND layers, to ensure that only one path is active at a time. If the USB bus is occupied, then userspace is notified that the path is busy. Several Qualcomm based chipsets can support USB audio offloading to a dedicated audio DSP, which can take over issuing transfers to the USB host controller. The intention is to reduce the load on the main processors in the SoC, and allow them to be placed into lower power modes. There are several parts to this design: 1. Adding ASoC binding layer 2. Create a USB backend for Q6DSP 3. Introduce XHCI interrupter support 4. Create vendor ops for the USB SND driver Adding ASoC binding layer: soc-usb: Intention is to treat a USB port similar to a headphone jack. The port is always present on the device, but cable/pin status can be enabled/disabled. Expose mechanisms for USB backend ASoC drivers to communicate with USB SND. Create a USB backend for Q6DSP: q6usb: Basic backend driver that will be responsible for maintaining the resources needed to initiate a playback stream using the Q6DSP. Will be the entity that checks to make sure the connected USB audio device supports the requested PCM format. If it does not, the PCM open call will fail, and userpsace ALSA can take action accordingly. Introduce XHCI interrupter support: XHCI HCD supports multiple interrupters, which allows for events to be routed to different event rings. This is determined by "Interrupter Target" field specified in Section "6.4.1.1 Normal TRB" of the XHCI specification. Events in the offloading case will be routed to an event ring that is assigned to the audio DSP. Create vendor ops for the USB SND driver: qc_audio_offload: This particular driver has several components associated with it: - QMI stream request handler - XHCI interrupter and resource management - audio DSP memory management When the audio DSP wants to enable a playback stream, the request is first received by the ASoC platform sound card. Depending on the selected route, ASoC will bring up the individual DAIs in the path. The Q6USB backend DAI will send an AFE port start command (with enabling the USB playback path), and the audio DSP will handle the request accordingly. Part of the AFE USB port start handling will have an exchange of control messages using the QMI protocol. The qc_audio_offload driver will populate the buffer information: - Event ring base address - EP transfer ring base address and pass it along to the audio DSP. All endpoint management will now be handed over to the DSP, and the main processor is not involved in transfers. Overall, implementing this feature will still expose separate sound card and PCM devices for both the platorm card and USB audio device: 0 [SM8250MTPWCD938]: sm8250 - SM8250-MTP-WCD9380-WSA8810-VA-D SM8250-MTP-WCD9380-WSA8810-VA-DMIC 1 [Audio ]: USB-Audio - USB Audio Generic USB Audio at usb-xhci-hcd.1.auto-1.4, high speed This is to ensure that userspace ALSA entities can decide which route to take when executing the audio playback. In the above, if card#1 is selected, then USB audio data will take the legacy path over the USB PCM drivers, etc... This feature was validated using: - tinymix: set/enable the multimedia path to route to USB backend - tinyplay: issue playback on platform card Mathias Nyman (1): xhci: Add support to allocate several interrupters Wesley Cheng (27): usb: xhci: Add XHCI APIs to support USB offloading usb: host: xhci-mem: Cleanup pending secondary event ring events ASoC: Add SOC USB APIs for adding an USB backend ASoC: dt-bindings: qcom,q6dsp-lpass-ports: Add USB_RX port ASoC: qcom: qdsp6: Introduce USB AFE port to q6dsp ASoC: qdsp6: q6afe: Increase APR timeout ASoC: qcom: Add USB backend ASoC driver for Q6 sound: usb: card: Introduce USB SND platform op callbacks sound: usb: Export USB SND APIs for modules dt-bindings: usb: dwc3: Add snps,num-hc-interrupters definition usb: dwc3: Add DT parameter to specify maximum number of interrupters usb: host: xhci-plat: Set XHCI max interrupters if property is present sound: usb: pcm: Export fixed rate check USB SND API sound: usb: Introduce QC USB SND offloading support sound: usb: card: Check for support for requested audio format sound: soc: soc-usb: Add PCM format check API for USB backend sound: soc: qcom: qusb6: Ensure PCM format is supported by USB audio device sound: usb: Prevent starting of audio stream if in use ASoC: dt-bindings: Add Q6USB backend bindings ASoC: dt-bindings: Update example for enabling USB offload on SM8250 ASoC: qcom: qdsp6: q6afe: Split USB AFE dev_token param into separate API sound: Pass USB SND card and PCM information to SOC USB sound: soc: qdsp6: Add SND kcontrol to select offload device sound: soc: qdsp6: Add SND kcontrol for fetching offload status sound: usb: qc_audio_offload: Use card and PCM index from QMI request sound: usb: card: Allow for rediscovery of connected USB SND devices sound: soc: soc-usb: Rediscover USB SND devices on USB port add .../bindings/sound/qcom,q6usb-dais.yaml | 49 + .../bindings/sound/qcom,sm8250.yaml | 15 + .../devicetree/bindings/usb/snps,dwc3.yaml | 13 + drivers/usb/dwc3/core.c | 12 + drivers/usb/dwc3/core.h | 2 + drivers/usb/dwc3/host.c | 5 +- drivers/usb/host/xhci-debugfs.c | 2 +- drivers/usb/host/xhci-mem.c | 171 +- drivers/usb/host/xhci-plat.c | 2 + drivers/usb/host/xhci-ring.c | 2 +- drivers/usb/host/xhci.c | 181 +- drivers/usb/host/xhci.h | 2 +- .../sound/qcom,q6dsp-lpass-ports.h | 1 + include/linux/usb/xhci-intr.h | 94 + include/sound/pcm_params.h | 4 + include/sound/q6usboffload.h | 20 + include/sound/soc-usb.h | 39 + sound/core/oss/pcm_oss.c | 58 - sound/core/pcm_lib.c | 65 + sound/soc/Makefile | 2 +- sound/soc/qcom/Kconfig | 4 + sound/soc/qcom/qdsp6/Makefile | 1 + sound/soc/qcom/qdsp6/q6afe-dai.c | 50 + sound/soc/qcom/qdsp6/q6afe.c | 208 +- sound/soc/qcom/qdsp6/q6afe.h | 47 +- sound/soc/qcom/qdsp6/q6dsp-lpass-ports.c | 23 + sound/soc/qcom/qdsp6/q6dsp-lpass-ports.h | 1 + sound/soc/qcom/qdsp6/q6routing.c | 9 + sound/soc/qcom/qdsp6/q6usb.c | 445 ++++ sound/soc/soc-usb.c | 197 ++ sound/usb/Kconfig | 14 + sound/usb/Makefile | 2 +- sound/usb/card.c | 85 + sound/usb/card.h | 31 + sound/usb/endpoint.c | 2 + sound/usb/helper.c | 1 + sound/usb/pcm.c | 29 +- sound/usb/pcm.h | 12 + sound/usb/qcom/Makefile | 2 + sound/usb/qcom/qc_audio_offload.c | 1790 +++++++++++++++++ sound/usb/qcom/usb_audio_qmi_v01.c | 892 ++++++++ sound/usb/qcom/usb_audio_qmi_v01.h | 162 ++ 42 files changed, 4646 insertions(+), 100 deletions(-) create mode 100644 Documentation/devicetree/bindings/sound/qcom,q6usb-dais.yaml create mode 100644 include/linux/usb/xhci-intr.h create mode 100644 include/sound/q6usboffload.h create mode 100644 include/sound/soc-usb.h create mode 100644 sound/soc/qcom/qdsp6/q6usb.c create mode 100644 sound/soc/soc-usb.c create mode 100644 sound/usb/qcom/Makefile create mode 100644 sound/usb/qcom/qc_audio_offload.c create mode 100644 sound/usb/qcom/usb_audio_qmi_v01.c create mode 100644 sound/usb/qcom/usb_audio_qmi_v01.h