| Message ID | 20240203023645.31105-49-quic_wcheng@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Introduce QC USB SND audio offloading support | expand |
On Sat, 03 Feb 2024 03:36:40 +0100, Wesley Cheng wrote: > > In order to allow userspace/applications know about USB offloading status, > expose a sound kcontrol that fetches information about which sound card > index is associated with the ASoC platform card supporting offloading. In > the USB audio offloading framework, the ASoC BE DAI link is the entity > responsible for registering to the SOC USB layer. SOC USB will expose more > details about the current offloading status, which includes the USB sound > card and USB PCM device indexes currently being used. > > Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com> The concept is understandable, but the control element name ("SNDUSB OFFLD playback available") looks non-intrusive and non-conformant. Use a bit more understandable name instead. This provides a card number where the offload driver is bound, and the name should indicate something about that. Also, about the implementation: > +static int > +snd_usb_offload_create_mixer(struct usb_mixer_interface *mixer, > + const struct snd_kcontrol_new *new_kctl) > +{ > + struct snd_kcontrol *kctl; > + struct usb_mixer_elem_info *elem; > + > + elem = kzalloc(sizeof(struct usb_mixer_elem_info), GFP_KERNEL); > + if (!elem) > + return -ENOMEM; > + > + elem->head.mixer = mixer; > + elem->val_type = USB_MIXER_S32; > + elem->control = 0; > + elem->head.id = 0; > + elem->channels = 1; > + > + kctl = snd_ctl_new1(new_kctl, elem); > + if (!kctl) { > + kfree(elem); > + return -ENOMEM; > + } > + kctl->private_free = snd_usb_mixer_elem_free; > + > + return snd_usb_mixer_add_control(&elem->head, kctl); This control has almost little to do with the standard USB interface, and it'll be much simpler if you create a raw control element. Pass the bus or the sysdev to private_data, and that's all you need in the get callback. Also, don't forget to set the proper access bits (it's read-only). thanks, Takashi
Hi Takashi, On 2/6/2024 4:57 AM, Takashi Iwai wrote: > On Sat, 03 Feb 2024 03:36:40 +0100, > Wesley Cheng wrote: >> >> In order to allow userspace/applications know about USB offloading status, >> expose a sound kcontrol that fetches information about which sound card >> index is associated with the ASoC platform card supporting offloading. In >> the USB audio offloading framework, the ASoC BE DAI link is the entity >> responsible for registering to the SOC USB layer. SOC USB will expose more >> details about the current offloading status, which includes the USB sound >> card and USB PCM device indexes currently being used. >> >> Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com> > > The concept is understandable, but the control element name ("SNDUSB > OFFLD playback available") looks non-intrusive and non-conformant. > Use a bit more understandable name instead. > > This provides a card number where the offload driver is bound, and the > name should indicate something about that. > Hmmm, does USB sound have a naming convention that it usually follows for mixer/control interfaces? For something that is more closely related, how about: "USB offload capable card" > Also, about the implementation: > >> +static int >> +snd_usb_offload_create_mixer(struct usb_mixer_interface *mixer, >> + const struct snd_kcontrol_new *new_kctl) >> +{ >> + struct snd_kcontrol *kctl; >> + struct usb_mixer_elem_info *elem; >> + >> + elem = kzalloc(sizeof(struct usb_mixer_elem_info), GFP_KERNEL); >> + if (!elem) >> + return -ENOMEM; >> + >> + elem->head.mixer = mixer; >> + elem->val_type = USB_MIXER_S32; >> + elem->control = 0; >> + elem->head.id = 0; >> + elem->channels = 1; >> + >> + kctl = snd_ctl_new1(new_kctl, elem); >> + if (!kctl) { >> + kfree(elem); >> + return -ENOMEM; >> + } >> + kctl->private_free = snd_usb_mixer_elem_free; >> + >> + return snd_usb_mixer_add_control(&elem->head, kctl); > > This control has almost little to do with the standard USB interface, > and it'll be much simpler if you create a raw control element. > Pass the bus or the sysdev to private_data, and that's all you need in > the get callback. > Sure, I'll remove the need to register over the SND USB mixer API, and just use the core SND control APIs. > Also, don't forget to set the proper access bits (it's read-only). > Thanks for pointing this out, will fix. Thanks Wesley Cheng > > thanks, > > Takashi
On Wed, 07 Feb 2024 02:24:46 +0100, Wesley Cheng wrote: > > Hi Takashi, > > On 2/6/2024 4:57 AM, Takashi Iwai wrote: > > On Sat, 03 Feb 2024 03:36:40 +0100, > > Wesley Cheng wrote: > >> > >> In order to allow userspace/applications know about USB offloading status, > >> expose a sound kcontrol that fetches information about which sound card > >> index is associated with the ASoC platform card supporting offloading. In > >> the USB audio offloading framework, the ASoC BE DAI link is the entity > >> responsible for registering to the SOC USB layer. SOC USB will expose more > >> details about the current offloading status, which includes the USB sound > >> card and USB PCM device indexes currently being used. > >> > >> Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com> > > > > The concept is understandable, but the control element name ("SNDUSB > > OFFLD playback available") looks non-intrusive and non-conformant. > > Use a bit more understandable name instead. > > > > This provides a card number where the offload driver is bound, and the > > name should indicate something about that. > > > > Hmmm, does USB sound have a naming convention that it usually follows > for mixer/control interfaces? The old rule is found in Documentation/sound/designs/control-names.rst (although the prefix and the suffix are often dropped for non-standard controls). > For something that is more closely related, how about: > "USB offload capable card" Yes, it looks better. But usually each word begins with an upper letter. Takashi
diff --git a/sound/usb/Kconfig b/sound/usb/Kconfig index 4c842fbe6365..3e7be258d0e3 100644 --- a/sound/usb/Kconfig +++ b/sound/usb/Kconfig @@ -176,10 +176,14 @@ config SND_BCD2000 To compile this driver as a module, choose M here: the module will be called snd-bcd2000. +config SND_USB_OFFLOAD_MIXER + bool + config SND_USB_AUDIO_QMI tristate "Qualcomm Audio Offload driver" depends on QCOM_QMI_HELPERS && SND_USB_AUDIO && USB_XHCI_SIDEBAND select SND_PCM + select SND_USB_OFFLOAD_MIXER help Say Y here to enable the Qualcomm USB audio offloading feature. diff --git a/sound/usb/Makefile b/sound/usb/Makefile index 246788268ddd..8c54660a11b0 100644 --- a/sound/usb/Makefile +++ b/sound/usb/Makefile @@ -22,6 +22,7 @@ snd-usb-audio-objs := card.o \ stream.o \ validate.o +snd-usb-audio-$(CONFIG_SND_USB_OFFLOAD_MIXER) += mixer_usb_offload.o snd-usb-audio-$(CONFIG_SND_USB_AUDIO_MIDI_V2) += midi2.o snd-usb-audio-$(CONFIG_SND_USB_AUDIO_USE_MEDIA_CONTROLLER) += media.o diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c index 409fc1164694..09229e623469 100644 --- a/sound/usb/mixer.c +++ b/sound/usb/mixer.c @@ -48,6 +48,7 @@ #include "mixer.h" #include "helper.h" #include "mixer_quirks.h" +#include "mixer_usb_offload.h" #include "power.h" #define MAX_ID_ELEMS 256 @@ -3609,6 +3610,10 @@ int snd_usb_create_mixer(struct snd_usb_audio *chip, int ctrlif) if (err < 0) goto _error; + err = snd_usb_offload_init_mixer(mixer); + if (err < 0) + goto _error; + err = snd_device_new(chip->card, SNDRV_DEV_CODEC, mixer, &dev_ops); if (err < 0) goto _error; diff --git a/sound/usb/mixer_usb_offload.c b/sound/usb/mixer_usb_offload.c new file mode 100644 index 000000000000..4e31c1848c53 --- /dev/null +++ b/sound/usb/mixer_usb_offload.c @@ -0,0 +1,82 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (c) 2022-2024 Qualcomm Innovation Center, Inc. All rights reserved. + */ + +#include <linux/usb.h> + +#include <sound/core.h> +#include <sound/control.h> +#include <sound/soc-usb.h> + +#include "card.h" +#include "mixer.h" +#include "mixer_usb_offload.h" +#include "usbaudio.h" + +static int +snd_usb_offload_create_mixer(struct usb_mixer_interface *mixer, + const struct snd_kcontrol_new *new_kctl) +{ + struct snd_kcontrol *kctl; + struct usb_mixer_elem_info *elem; + + elem = kzalloc(sizeof(struct usb_mixer_elem_info), GFP_KERNEL); + if (!elem) + return -ENOMEM; + + elem->head.mixer = mixer; + elem->val_type = USB_MIXER_S32; + elem->control = 0; + elem->head.id = 0; + elem->channels = 1; + + kctl = snd_ctl_new1(new_kctl, elem); + if (!kctl) { + kfree(elem); + return -ENOMEM; + } + kctl->private_free = snd_usb_mixer_elem_free; + + return snd_usb_mixer_add_control(&elem->head, kctl); +} + +static int +snd_usb_offload_available_get(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct usb_mixer_elem_list *list = snd_kcontrol_chip(kcontrol); + struct usb_mixer_interface *mixer = list->mixer; + struct snd_usb_audio *chip = mixer->chip; + struct usb_device *udev = chip->dev; + int ret; + + ret = snd_soc_usb_device_offload_available(udev->bus->sysdev); + ucontrol->value.integer.value[0] = ret < 0 ? -1 : ret; + + return ret < 0 ? ret : 0; +} + +static int snd_usb_offload_available_info(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_info *uinfo) +{ + uinfo->type = SNDRV_CTL_ELEM_TYPE_INTEGER; + uinfo->count = 1; + uinfo->value.integer.min = -1; + uinfo->value.integer.max = SNDRV_CARDS; + + return 0; +} + +static const struct snd_kcontrol_new snd_usb_offload_available_ctl = { + .iface = SNDRV_CTL_ELEM_IFACE_CARD, + .name = "SNDUSB OFFLD playback available", + .info = snd_usb_offload_available_info, + .get = snd_usb_offload_available_get, +}; + +int snd_usb_offload_init_mixer(struct usb_mixer_interface *mixer) +{ + return snd_usb_offload_create_mixer(mixer, &snd_usb_offload_available_ctl); +} +EXPORT_SYMBOL_GPL(snd_usb_offload_init_mixer); diff --git a/sound/usb/mixer_usb_offload.h b/sound/usb/mixer_usb_offload.h new file mode 100644 index 000000000000..fb88e872d8fa --- /dev/null +++ b/sound/usb/mixer_usb_offload.h @@ -0,0 +1,17 @@ +/* SPDX-License-Identifier: GPL-2.0 + * + * Copyright (c) 2022-2024 Qualcomm Innovation Center, Inc. All rights reserved. + */ + +#ifndef __USB_OFFLOAD_MIXER_H +#define __USB_OFFLOAD_MIXER_H + +#if IS_ENABLED(CONFIG_SND_USB_OFFLOAD_MIXER) +int snd_usb_offload_init_mixer(struct usb_mixer_interface *mixer); +#else +static int snd_usb_offload_init_mixer(struct usb_mixer_interface *mixer) +{ + return 0; +} +#endif +#endif /* __USB_OFFLOAD_MIXER_H */
In order to allow userspace/applications know about USB offloading status, expose a sound kcontrol that fetches information about which sound card index is associated with the ASoC platform card supporting offloading. In the USB audio offloading framework, the ASoC BE DAI link is the entity responsible for registering to the SOC USB layer. SOC USB will expose more details about the current offloading status, which includes the USB sound card and USB PCM device indexes currently being used. Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com> --- sound/usb/Kconfig | 4 ++ sound/usb/Makefile | 1 + sound/usb/mixer.c | 5 +++ sound/usb/mixer_usb_offload.c | 82 +++++++++++++++++++++++++++++++++++ sound/usb/mixer_usb_offload.h | 17 ++++++++ 5 files changed, 109 insertions(+) create mode 100644 sound/usb/mixer_usb_offload.c create mode 100644 sound/usb/mixer_usb_offload.h