From: Takashi Iwai <tiwai@suse.de> To: Wesley Cheng <quic_wcheng@quicinc.com> Cc: <srinivas.kandagatla@linaro.org>, <mathias.nyman@intel.com>, <perex@perex.cz>, <broonie@kernel.org>, <lgirdwood@gmail.com>, <andersson@kernel.org>, <krzysztof.kozlowski+dt@linaro.org>, <gregkh@linuxfoundation.org>, <Thinh.Nguyen@synopsys.com>, <bgoswami@quicinc.com>, <tiwai@suse.com>, <robh+dt@kernel.org>, <agross@kernel.org>, <linux-kernel@vger.kernel.org>, <linux-arm-msm@vger.kernel.org>, <alsa-devel@alsa-project.org>, <devicetree@vger.kernel.org>, <linux-usb@vger.kernel.org>, <quic_jackp@quicinc.com>, <quic_plai@quicinc.com> Subject: Re: [RFC PATCH 09/14] sound: usb: Introduce QC USB SND offloading support Date: Mon, 02 Jan 2023 18:28:34 +0100 [thread overview] Message-ID: <87edscsv5p.wl-tiwai@suse.de> (raw) In-Reply-To: <20221223233200.26089-10-quic_wcheng@quicinc.com> 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@quicinc.com> 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. Also, since both drivers are asynchronous, we may need some proper locking. 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. > +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? > +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. thanks, Takashi
WARNING: multiple messages have this Message-ID (diff)
From: Takashi Iwai <tiwai@suse.de> To: Wesley Cheng <quic_wcheng@quicinc.com> Cc: linux-arm-msm@vger.kernel.org, devicetree@vger.kernel.org, alsa-devel@alsa-project.org, linux-usb@vger.kernel.org, bgoswami@quicinc.com, mathias.nyman@intel.com, gregkh@linuxfoundation.org, andersson@kernel.org, tiwai@suse.com, lgirdwood@gmail.com, robh+dt@kernel.org, broonie@kernel.org, srinivas.kandagatla@linaro.org, agross@kernel.org, krzysztof.kozlowski+dt@linaro.org, Thinh.Nguyen@synopsys.com, quic_plai@quicinc.com, linux-kernel@vger.kernel.org, quic_jackp@quicinc.com Subject: Re: [RFC PATCH 09/14] sound: usb: Introduce QC USB SND offloading support Date: Mon, 02 Jan 2023 18:28:34 +0100 [thread overview] Message-ID: <87edscsv5p.wl-tiwai@suse.de> (raw) In-Reply-To: <20221223233200.26089-10-quic_wcheng@quicinc.com> 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@quicinc.com> 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. Also, since both drivers are asynchronous, we may need some proper locking. 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. > +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? > +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. thanks, Takashi
next prev parent reply other threads:[~2023-01-02 17:28 UTC|newest] Thread overview: 171+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-12-23 23:31 [RFC PATCH 00/14] Introduce QC USB SND audio offloading support Wesley Cheng 2022-12-23 23:31 ` Wesley Cheng 2022-12-23 23:31 ` [RFC PATCH 01/14] ASoC: Add SOC USB APIs for adding an USB backend Wesley Cheng 2022-12-23 23:31 ` Wesley Cheng 2022-12-24 6:48 ` Greg KH 2022-12-24 6:48 ` Greg KH 2022-12-23 23:31 ` [RFC PATCH 02/14] ASoC: qcom: qdsp6: Introduce USB AFE port to q6dsp Wesley Cheng 2022-12-23 23:31 ` Wesley Cheng 2022-12-24 1:37 ` kernel test robot 2023-01-04 23:33 ` Pierre-Louis Bossart 2023-01-06 1:05 ` Wesley Cheng 2023-01-06 1:05 ` Wesley Cheng 2023-01-06 16:09 ` Pierre-Louis Bossart 2023-01-07 0:51 ` Wesley Cheng 2023-01-07 0:51 ` Wesley Cheng 2023-01-05 18:09 ` Krzysztof Kozlowski 2023-01-05 18:09 ` Krzysztof Kozlowski 2023-01-06 1:32 ` Wesley Cheng 2023-01-06 1:32 ` Wesley Cheng 2022-12-23 23:31 ` [RFC PATCH 03/14] ASoC: qcom: Add USB backend ASoC driver for Q6 Wesley Cheng 2022-12-23 23:31 ` Wesley Cheng 2022-12-24 2:17 ` kernel test robot 2022-12-24 2:17 ` kernel test robot 2022-12-24 9:02 ` Greg KH 2022-12-24 9:02 ` Greg KH 2022-12-27 13:04 ` Mark Brown 2022-12-27 13:04 ` Mark Brown 2022-12-27 13:45 ` Greg KH 2022-12-27 13:45 ` Greg KH 2022-12-27 14:02 ` Takashi Iwai 2022-12-27 14:02 ` Takashi Iwai 2022-12-27 14:11 ` Mark Brown 2022-12-27 14:11 ` Mark Brown 2022-12-27 15:11 ` Mark Brown 2022-12-27 15:11 ` Mark Brown 2022-12-27 21:06 ` Wesley Cheng 2022-12-27 21:06 ` Wesley Cheng 2022-12-27 21:07 ` Wesley Cheng 2022-12-27 21:07 ` Wesley Cheng 2023-01-04 23:41 ` Pierre-Louis Bossart 2023-01-06 1:05 ` Wesley Cheng 2023-01-06 1:05 ` Wesley Cheng 2023-01-06 16:16 ` Pierre-Louis Bossart 2022-12-23 23:31 ` [RFC PATCH 04/14] sound: usb: card: Introduce USB SND vendor op callbacks Wesley Cheng 2022-12-23 23:31 ` Wesley Cheng 2022-12-24 11:03 ` Dmitry Baryshkov 2022-12-24 11:03 ` Dmitry Baryshkov 2022-12-27 21:07 ` Wesley Cheng 2022-12-27 21:07 ` Wesley Cheng 2022-12-27 21:33 ` Dmitry Baryshkov 2022-12-27 21:33 ` Dmitry Baryshkov 2022-12-29 13:49 ` Oliver Neukum 2022-12-29 13:49 ` Oliver Neukum 2022-12-29 14:20 ` Takashi Iwai 2022-12-29 14:20 ` Takashi Iwai 2022-12-30 7:10 ` Wesley Cheng 2022-12-30 7:10 ` Wesley Cheng 2023-01-03 12:20 ` Oliver Neukum 2023-01-03 12:20 ` Oliver Neukum 2023-01-03 12:49 ` Takashi Iwai 2023-01-03 12:49 ` Takashi Iwai 2023-01-03 23:45 ` Wesley Cheng 2023-01-03 23:45 ` Wesley Cheng 2022-12-23 23:31 ` [RFC PATCH 05/14] sound: usb: Export USB SND APIs for modules Wesley Cheng 2022-12-23 23:31 ` Wesley Cheng 2022-12-24 6:48 ` Greg KH 2022-12-24 6:48 ` Greg KH 2022-12-23 23:31 ` [RFC PATCH 06/14] usb: core: hcd: Introduce USB HCD APIs for interrupter management Wesley Cheng 2022-12-23 23:31 ` Wesley Cheng 2022-12-24 8:54 ` Greg KH 2022-12-24 8:54 ` Greg KH 2022-12-27 21:13 ` Wesley Cheng 2022-12-27 21:13 ` Wesley Cheng 2022-12-24 15:29 ` Alan Stern 2022-12-24 15:29 ` Alan Stern 2022-12-27 21:07 ` Wesley Cheng 2022-12-27 21:07 ` Wesley Cheng 2022-12-28 8:59 ` Oliver Neukum 2022-12-28 8:59 ` Oliver Neukum 2022-12-28 15:16 ` Alan Stern 2022-12-28 15:16 ` Alan Stern 2022-12-28 20:31 ` Wesley Cheng 2022-12-28 20:31 ` Wesley Cheng 2022-12-29 1:41 ` Alan Stern 2022-12-29 1:41 ` Alan Stern 2022-12-23 23:31 ` [RFC PATCH 07/14] usb: host: xhci: Add XHCI secondary interrupter support Wesley Cheng 2022-12-23 23:31 ` Wesley Cheng 2022-12-24 5:59 ` kernel test robot 2022-12-24 8:55 ` Greg KH 2022-12-24 8:55 ` Greg KH 2022-12-28 15:47 ` Mathias Nyman 2022-12-28 15:47 ` Mathias Nyman 2022-12-29 21:14 ` Wesley Cheng 2022-12-29 21:14 ` Wesley Cheng 2023-01-02 16:38 ` Mathias Nyman 2023-01-02 16:38 ` Mathias Nyman 2023-01-09 20:24 ` Wesley Cheng 2023-01-09 20:24 ` Wesley Cheng 2023-01-10 19:47 ` Mathias Nyman 2023-01-10 19:47 ` Mathias Nyman 2023-01-10 20:03 ` Wesley Cheng 2023-01-10 20:03 ` Wesley Cheng 2023-01-11 3:11 ` Wesley Cheng 2023-01-11 3:11 ` Wesley Cheng 2023-01-12 9:24 ` Mathias Nyman 2023-01-12 9:24 ` Mathias Nyman 2023-01-13 0:34 ` Wesley Cheng 2023-01-13 0:34 ` Wesley Cheng 2022-12-23 23:31 ` [RFC PATCH 08/14] usb: dwc3: Add DT parameter to specify maximum number of interrupters Wesley Cheng 2022-12-23 23:31 ` Wesley Cheng 2022-12-24 11:13 ` Dmitry Baryshkov 2022-12-24 11:13 ` Dmitry Baryshkov 2022-12-26 12:28 ` Krzysztof Kozlowski 2022-12-26 12:28 ` Krzysztof Kozlowski 2022-12-27 21:06 ` Wesley Cheng 2022-12-27 21:06 ` Wesley Cheng 2022-12-23 23:31 ` [RFC PATCH 09/14] sound: usb: Introduce QC USB SND offloading support Wesley Cheng 2022-12-23 23:31 ` Wesley Cheng 2022-12-24 2:27 ` kernel test robot 2023-01-02 17:28 ` Takashi Iwai [this message] 2023-01-02 17:28 ` Takashi Iwai 2023-01-04 22:38 ` Wesley Cheng 2023-01-04 22:38 ` Wesley Cheng 2023-01-04 23:51 ` Pierre-Louis Bossart 2023-01-06 1:06 ` Wesley Cheng 2023-01-06 1:06 ` Wesley Cheng 2022-12-23 23:31 ` [RFC PATCH 10/14] sound: usb: card: Check for support for requested audio format Wesley Cheng 2022-12-23 23:31 ` Wesley Cheng 2022-12-24 8:59 ` Greg KH 2022-12-24 8:59 ` Greg KH 2022-12-27 21:07 ` Wesley Cheng 2022-12-27 21:07 ` Wesley Cheng 2022-12-27 20:31 ` kernel test robot 2022-12-23 23:31 ` [RFC PATCH 11/14] sound: soc: soc-usb: Add PCM format check API for USB backend Wesley Cheng 2022-12-23 23:31 ` Wesley Cheng 2022-12-24 2:17 ` kernel test robot 2022-12-24 2:17 ` kernel test robot 2022-12-23 23:31 ` [RFC PATCH 12/14] sound: soc: qcom: qusb6: Ensure PCM format is supported by USB audio device Wesley Cheng 2022-12-23 23:31 ` Wesley Cheng 2022-12-24 8:19 ` Sergey Shtylyov 2022-12-24 8:19 ` Sergey Shtylyov 2022-12-24 8:50 ` Wesley Cheng 2022-12-24 8:50 ` Wesley Cheng 2023-01-03 17:44 ` Mark Brown 2023-01-03 17:44 ` Mark Brown 2022-12-23 23:31 ` [RFC PATCH 13/14] ASoC: dt-bindings: Add Q6USB backend bindings Wesley Cheng 2022-12-23 23:31 ` Wesley Cheng 2022-12-26 12:25 ` Krzysztof Kozlowski 2022-12-26 12:25 ` Krzysztof Kozlowski 2022-12-23 23:32 ` [RFC PATCH 14/14] ASoC: dt-bindings: Update example for enabling USB offload on SM8250 Wesley Cheng 2022-12-23 23:32 ` Wesley Cheng 2022-12-26 12:27 ` Krzysztof Kozlowski 2022-12-26 12:27 ` Krzysztof Kozlowski 2023-01-03 17:46 ` Mark Brown 2023-01-03 17:46 ` Mark Brown 2023-01-05 18:09 ` Krzysztof Kozlowski 2023-01-05 18:09 ` Krzysztof Kozlowski 2023-01-04 0:46 ` Rob Herring 2023-01-04 0:46 ` Rob Herring 2022-12-24 6:45 ` [RFC PATCH 00/14] Introduce QC USB SND audio offloading support Greg KH 2022-12-24 6:45 ` Greg KH 2022-12-24 8:49 ` Wesley Cheng 2022-12-24 8:49 ` Wesley Cheng 2022-12-27 14:36 ` Mark Brown 2022-12-27 14:36 ` Mark Brown 2023-01-04 23:19 ` Pierre-Louis Bossart 2023-01-06 1:05 ` Wesley Cheng 2023-01-06 1:05 ` Wesley Cheng 2023-01-06 15:57 ` Pierre-Louis Bossart 2023-01-07 0:46 ` Wesley Cheng 2023-01-07 0:46 ` Wesley Cheng
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=87edscsv5p.wl-tiwai@suse.de \ --to=tiwai@suse.de \ --cc=Thinh.Nguyen@synopsys.com \ --cc=agross@kernel.org \ --cc=alsa-devel@alsa-project.org \ --cc=andersson@kernel.org \ --cc=bgoswami@quicinc.com \ --cc=broonie@kernel.org \ --cc=devicetree@vger.kernel.org \ --cc=gregkh@linuxfoundation.org \ --cc=krzysztof.kozlowski+dt@linaro.org \ --cc=lgirdwood@gmail.com \ --cc=linux-arm-msm@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-usb@vger.kernel.org \ --cc=mathias.nyman@intel.com \ --cc=perex@perex.cz \ --cc=quic_jackp@quicinc.com \ --cc=quic_plai@quicinc.com \ --cc=quic_wcheng@quicinc.com \ --cc=robh+dt@kernel.org \ --cc=srinivas.kandagatla@linaro.org \ --cc=tiwai@suse.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.