Hi Takashi,
On 1/2/2023 9:28 AM, Takashi Iwai wrote:
On Sat, 24 Dec 2022 00:31:55 +0100,
Wesley Cheng wrote:
Several Qualcomm SoCs have a dedicated audio DSP, which has the ability to
support USB sound devices. This vendor driver will implement the required
handshaking with the DSP, in order to pass along required resources that
will be utilized by the DSP's USB SW. The communication channel used for
this handshaking will be using the QMI protocol. Required resources
include:
- Allocated secondary event ring address
- EP transfer ring address
- Interrupter number
The above information will allow for the audio DSP to execute USB transfers
over the USB bus. It will also be able to support devices that have an
implicit feedback and sync endpoint as well. Offloading these data
transfers will allow the main/applications processor to enter lower CPU
power modes, and sustain a longer duration in those modes.
Signed-off-by: Wesley Cheng <quic_wcheng@xxxxxxxxxxx>
Hmm, this must be the main part that works to bypass the normal USB
packet handling in USB audio driver but hooks to the own offload one,
but there is no description how to take over and manage.
A missing "big picture" makes it difficult to understand and review.
Technically, we are not taking over the functionality of the USB SND, as
we still want the normal path to be accessible in case there is an audio
profile/format that can't be supported by the audio DSP. I can add some
more information on how this offload driver co-exists with the USB SND.
Also, since both drivers are asynchronous, we may need some proper
locking.
Yes, I think locking is needed in some places. Will add that in the
next revision.
More on the code change:
+static int snd_interval_refine_set(struct snd_interval *i, unsigned int val)
+{
+ struct snd_interval t;
+
+ t.empty = 0;
+ t.min = t.max = val;
+ t.openmin = t.openmax = 0;
+ t.integer = 1;
+ return snd_interval_refine(i, &t);
+}
+
+static int _snd_pcm_hw_param_set(struct snd_pcm_hw_params *params,
+ snd_pcm_hw_param_t var, unsigned int val,
+ int dir)
+{
+ int changed;
+
+ if (hw_is_mask(var)) {
+ struct snd_mask *m = hw_param_mask(params, var);
+
+ if (val == 0 && dir < 0) {
+ changed = -EINVAL;
+ snd_mask_none(m);
+ } else {
+ if (dir > 0)
+ val++;
+ else if (dir < 0)
+ val--;
+ changed = snd_mask_refine_set(
+ hw_param_mask(params, var), val);
+ }
+ } else if (hw_is_interval(var)) {
+ struct snd_interval *i = hw_param_interval(params, var);
+
+ if (val == 0 && dir < 0) {
+ changed = -EINVAL;
+ snd_interval_none(i);
+ } else if (dir == 0)
+ changed = snd_interval_refine_set(i, val);
+ else {
+ struct snd_interval t;
+
+ t.openmin = 1;
+ t.openmax = 1;
+ t.empty = 0;
+ t.integer = 0;
+ if (dir < 0) {
+ t.min = val - 1;
+ t.max = val;
+ } else {
+ t.min = val;
+ t.max = val+1;
+ }
+ changed = snd_interval_refine(i, &t);
+ }
+ } else
+ return -EINVAL;
+ if (changed) {
+ params->cmask |= 1 << var;
+ params->rmask |= 1 << var;
+ }
+ return changed;
+}
Those are taken from sound/core/oss/pcm_oss.c? We may put to the
common PCM helper instead of duplication.
Sure, I can do that.
+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);
+}
+
+static int enable_audio_stream(struct snd_usb_substream *subs,
+ snd_pcm_format_t pcm_format,
+ unsigned int channels, unsigned int cur_rate,
+ int datainterval)
+{
+ struct snd_usb_audio *chip = subs->stream->chip;
+ struct snd_pcm_hw_params params;
+ const struct audioformat *fmt;
+ int ret;
+
+ _snd_pcm_hw_params_any(¶ms);
+ _snd_pcm_hw_param_set(¶ms, SNDRV_PCM_HW_PARAM_FORMAT,
+ pcm_format, 0);
+ _snd_pcm_hw_param_set(¶ms, SNDRV_PCM_HW_PARAM_CHANNELS,
+ channels, 0);
+ _snd_pcm_hw_param_set(¶ms, SNDRV_PCM_HW_PARAM_RATE,
+ cur_rate, 0);
What about other parameters like period / buffer sizes?
I don't think we will need those parameters on the audio DSP. The
"params" here is used to pass the pcm format into the qmi response.
+struct qmi_uaudio_stream_req_msg_v01 {
+ u8 enable;
+ u32 usb_token;
+ u8 audio_format_valid;
+ u32 audio_format;
+ u8 number_of_ch_valid;
+ u32 number_of_ch;
+ u8 bit_rate_valid;
+ u32 bit_rate;
+ u8 xfer_buff_size_valid;
+ u32 xfer_buff_size;
+ u8 service_interval_valid;
+ u32 service_interval;
+};
Are this and the other structs a part of DSP ABI?
Or is it a definition only used in kernel? I'm asking because
__packed attribute is required for most of ABI definitions with
different field types.
This would be in the kernel only.
Thanks
Wesley Cheng
