Hi Pierre,
On 8/1/2024 2:02 AM, Pierre-Louis Bossart wrote:
>
>> +ifneq ($(CONFIG_SND_USB_QC_OFFLOAD_MIXER),)
>> +snd-usb-audio-qmi-objs += mixer_usb_offload.o
>> +endif
>> \ No newline at end of file
> add one?
>
>> diff --git a/sound/usb/qcom/mixer_usb_offload.c b/sound/usb/qcom/mixer_usb_offload.c
>> new file mode 100644
>> index 000000000000..c00770400c67
>> --- /dev/null
>> +++ b/sound/usb/qcom/mixer_usb_offload.c
>> @@ -0,0 +1,101 @@
>> +// 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 "../usbaudio.h"
>> +
>> +#include "mixer_usb_offload.h"
>> +
>> +#define PCM_IDX(n) (n & 0xffff)
>> +#define CARD_IDX(n) (n >> 16)
>> +
>> +static int
>> +snd_usb_offload_route_get(struct snd_kcontrol *kcontrol,
>> + struct snd_ctl_elem_value *ucontrol)
>> +{
>> + struct device *sysdev = snd_kcontrol_chip(kcontrol);
>> + int card;
>> + int pcm;
>> +
>> + card = soc_usb_get_offload_device(sysdev, CARD_IDX(kcontrol->private_value),
>> + PCM_IDX(kcontrol->private_value),
>> + SND_SOC_USB_KCTL_CARD_ROUTE);
>> +
>> + pcm = soc_usb_get_offload_device(sysdev, CARD_IDX(kcontrol->private_value),
>> + PCM_IDX(kcontrol->private_value),
>> + SND_SOC_USB_KCTL_PCM_ROUTE);
>> + if (card < 0 || pcm < 0) {
>> + card = -1;
>> + pcm = -1;
>> + }
>> +
>> + ucontrol->value.integer.value[0] = card;
>> + ucontrol->value.integer.value[1] = pcm;
>> +
>> + return 0;
>> +}
> see my earlier comment, should those two calls be collapsed to return
> all the information in one shot?
>
>> +
>> +static int snd_usb_offload_route_info(struct snd_kcontrol *kcontrol,
>> + struct snd_ctl_elem_info *uinfo)
>> +{
>> + uinfo->type = SNDRV_CTL_ELEM_TYPE_INTEGER;
>> + uinfo->count = 2;
>> + uinfo->value.integer.min = -1;
>> + /* Arbitrary max value, as there is no 'limit' on number of PCM devices */
>> + uinfo->value.integer.max = 0xff;
>> +
>> + return 0;
>> +}
>> +
>> +static struct snd_kcontrol_new snd_usb_offload_mapped_ctl = {
>> + .iface = SNDRV_CTL_ELEM_IFACE_CARD,
>> + .access = SNDRV_CTL_ELEM_ACCESS_READ,
>> + .info = snd_usb_offload_route_info,
>> + .get = snd_usb_offload_route_get,
>> +};
>> +
>> +/**
>> + * snd_usb_offload_create_ctl() - Add USB offload bounded mixer
>> + * @chip - USB SND chip device
>> + *
>> + * Creates a sound control for a USB audio device, so that applications can
>> + * query for if there is an available USB audio offload path, and which
>> + * card is managing it.
>> + */
>> +int snd_usb_offload_create_ctl(struct snd_usb_audio *chip)
>> +{
>> + struct usb_device *udev = chip->dev;
>> + struct snd_kcontrol_new *chip_kctl;
>> + struct snd_usb_stream *as;
>> + char ctl_name[37];
>> + int ret;
>> +
>> + list_for_each_entry(as, &chip->pcm_list, list) {
>> + chip_kctl = &snd_usb_offload_mapped_ctl;
>> + chip_kctl->count = 1;
>> + /*
>> + * Store the associated USB SND card number and PCM index for
>> + * the kctl.
>> + */
>> + chip_kctl->private_value = as->pcm_index |
>> + chip->card->number << 16;
>> + sprintf(ctl_name, "USB Offload Playback Route PCM#%d",
>> + as->pcm_index);
>> + chip_kctl->name = ctl_name;
>> + ret = snd_ctl_add(chip->card, snd_ctl_new1(chip_kctl,
>> + udev->bus->sysdev));
>> + if (ret < 0)
>> + break;
>> + }
>> +
>> + return ret;
Hi Pierre,
> None of this looks Qualcomm-specific, shouldn't this be part of the
> soc_usb framework instead of being added in the qcom/ stuff?
Started working on this particular comment, and there are some things that needs to be considered if we moved this into SOC USB:
1. We do save the reference to the USB BE DAI link within the USB DT node, which can be fetched/referenced based on sysdev. However, I'm not sure if everyone would potentially follow that way.
2. I tried a few implementations of adding a new SOC USB API, and the argument list was a bit long, because I didn't want to directly reference the usb_chip.
Sorry for the delay, but I wanted to give a good stab at implementing this before bringing up the implications. It is possible, but definitely not as clean as how we have it now IMO.
Thanks
Wesley Cheng
