| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <170a622b-33f5-bd8f-b473-815390fa67ef@quicinc.com>
Date: Mon, 7 Apr 2025 16:01:32 -0700
From: Wesley Cheng <quic_wcheng@...cinc.com>
To: Stephan Gerhold <stephan.gerhold@...aro.org>
CC: <srinivas.kandagatla@...aro.org>, <mathias.nyman@...el.com>,
<perex@...ex.cz>, <conor+dt@...nel.org>, <dmitry.torokhov@...il.com>,
<corbet@....net>, <broonie@...nel.org>, <lgirdwood@...il.com>,
<krzk+dt@...nel.org>, <pierre-louis.bossart@...ux.intel.com>,
<Thinh.Nguyen@...opsys.com>, <tiwai@...e.com>, <robh@...nel.org>,
<gregkh@...uxfoundation.org>, <linux-kernel@...r.kernel.org>,
<devicetree@...r.kernel.org>, <linux-sound@...r.kernel.org>,
<linux-usb@...r.kernel.org>, <linux-input@...r.kernel.org>,
<linux-arm-msm@...r.kernel.org>, <linux-doc@...r.kernel.org>
Subject: Re: [PATCH v37 22/31] ASoC: qcom: qdsp6: Introduce USB AFE port to
q6dsp
Hi Stephan,
On 4/4/2025 2:24 AM, Stephan Gerhold wrote:
> On Thu, Apr 03, 2025 at 05:27:19PM -0700, Wesley Cheng wrote:
>> The QC ADSP is able to support USB playback endpoints, so that the main
>> application processor can be placed into lower CPU power modes. This adds
>> the required AFE port configurations and port start command to start an
>> audio session.
>>
>> Specifically, the QC ADSP can support all potential endpoints that are
>> exposed by the audio data interface. This includes isochronous data
>> endpoints, in either synchronous mode or asynchronous mode. In the latter
>> case both implicit or explicit feedback endpoints are supported. The size
>> of audio samples sent per USB frame (microframe) will be adjusted based on
>> information received on the feedback endpoint.
>>
>> Some pre-requisites are needed before issuing the AFE port start command,
>> such as setting the USB AFE dev_token. This carries information about the
>> available USB SND cards and PCM devices that have been discovered on the
>> USB bus. The dev_token field is used by the audio DSP to notify the USB
>> offload driver of which card and PCM index to enable playback on.
>>
>> Signed-off-by: Wesley Cheng <quic_wcheng@...cinc.com>
>> ---
>> sound/soc/qcom/qdsp6/q6afe-dai.c | 60 +++++++
>> sound/soc/qcom/qdsp6/q6afe.c | 192 ++++++++++++++++++++++-
>> sound/soc/qcom/qdsp6/q6afe.h | 36 ++++-
>> sound/soc/qcom/qdsp6/q6dsp-lpass-ports.c | 23 +++
>> sound/soc/qcom/qdsp6/q6dsp-lpass-ports.h | 1 +
>> sound/soc/qcom/qdsp6/q6routing.c | 10 +-
>> 6 files changed, 319 insertions(+), 3 deletions(-)
>>
>> diff --git a/sound/soc/qcom/qdsp6/q6afe-dai.c b/sound/soc/qcom/qdsp6/q6afe-dai.c
>> index 7d9628cda875..0f47aadaabe1 100644
>> --- a/sound/soc/qcom/qdsp6/q6afe-dai.c
>> +++ b/sound/soc/qcom/qdsp6/q6afe-dai.c
>> [...]
>> +static int afe_port_send_usb_params(struct q6afe_port *port, struct q6afe_usb_cfg *cfg)
>> +{
>> + union afe_port_config *pcfg = &port->port_cfg;
>> + struct afe_param_id_usb_audio_dev_lpcm_fmt lpcm_fmt;
>> + struct afe_param_id_usb_audio_svc_interval svc_int;
>> + int ret;
>> +
>> + if (!pcfg) {
>> + dev_err(port->afe->dev, "%s: Error, no configuration data\n", __func__);
>> + ret = -EINVAL;
>> + goto exit;
>
> Nitpick: drop the goto here, just do "return -EINVAL;"
>
>> + }
>> +
>> + memset(&lpcm_fmt, 0, sizeof(lpcm_fmt));
>> + memset(&svc_int, 0, sizeof(svc_int));
>> +
>> + lpcm_fmt.cfg_minor_version = AFE_API_MINOR_VERSION_USB_AUDIO_CONFIG;
>> + lpcm_fmt.endian = pcfg->usb_cfg.endian;
>> + ret = q6afe_port_set_param_v2(port, &lpcm_fmt,
>> + AFE_PARAM_ID_USB_AUDIO_DEV_LPCM_FMT,
>> + AFE_MODULE_AUDIO_DEV_INTERFACE, sizeof(lpcm_fmt));
>> + if (ret) {
>> + dev_err(port->afe->dev, "%s: AFE device param cmd LPCM_FMT failed %d\n",
>> + __func__, ret);
>> + goto exit;
>
> return ret;
>
>> + }
>> +
>> + svc_int.cfg_minor_version = AFE_API_MINOR_VERSION_USB_AUDIO_CONFIG;
>> + svc_int.svc_interval = pcfg->usb_cfg.service_interval;
>> + ret = q6afe_port_set_param_v2(port, &svc_int,
>> + AFE_PARAM_ID_USB_AUDIO_SVC_INTERVAL,
>> + AFE_MODULE_AUDIO_DEV_INTERFACE, sizeof(svc_int));
>> + if (ret)
>> + dev_err(port->afe->dev, "%s: AFE device param cmd svc_interval failed %d\n",
>> + __func__, ret);
>> +
>> +exit:
>
> drop
>
Done
>> + return ret;
>> +}
>> +
>> [...]
>> diff --git a/sound/soc/qcom/qdsp6/q6routing.c b/sound/soc/qcom/qdsp6/q6routing.c
>> index 90228699ba7d..0def036ed3c9 100644
>> --- a/sound/soc/qcom/qdsp6/q6routing.c
>> +++ b/sound/soc/qcom/qdsp6/q6routing.c
>> @@ -435,6 +435,7 @@ static struct session_data *get_session_from_id(struct msm_routing_data *data,
>>
>> return NULL;
>> }
>> +
>> /**
>> * q6routing_stream_close() - Deregister a stream
>> *
>> @@ -515,6 +516,9 @@ static int msm_routing_put_audio_mixer(struct snd_kcontrol *kcontrol,
>> return 1;
>> }
>>
>> +static const struct snd_kcontrol_new usb_rx_mixer_controls[] = {
>> + Q6ROUTING_RX_MIXERS(USB_RX) };
>> +
>> static const struct snd_kcontrol_new hdmi_mixer_controls[] = {
>> Q6ROUTING_RX_MIXERS(HDMI_RX) };
>>
>> @@ -933,6 +937,9 @@ static const struct snd_soc_dapm_widget msm_qdsp6_widgets[] = {
>> SND_SOC_DAPM_MIXER("RX_CODEC_DMA_RX_7 Audio Mixer", SND_SOC_NOPM, 0, 0,
>> rx_codec_dma_rx_7_mixer_controls,
>> ARRAY_SIZE(rx_codec_dma_rx_7_mixer_controls)),
>> + SND_SOC_DAPM_MIXER("USB Mixer", SND_SOC_NOPM, 0, 0,
>
> As I wrote on v36:
>
> I think it would be more clear if you call this "USB_RX Audio Mixer"
> instead for consistency with the other playback mixers. This would also
> avoid confusion later when USB_TX is added in addition to USB_RX.
>
> The "Audio" part in the name is redundant, but looks like all the other
> playback mixers have it as well ...
>
>
>> + usb_rx_mixer_controls,
>> + ARRAY_SIZE(usb_rx_mixer_controls)),
>> SND_SOC_DAPM_MIXER("MultiMedia1 Mixer", SND_SOC_NOPM, 0, 0,
>> mmul1_mixer_controls, ARRAY_SIZE(mmul1_mixer_controls)),
>> SND_SOC_DAPM_MIXER("MultiMedia2 Mixer", SND_SOC_NOPM, 0, 0,
>> @@ -949,7 +956,6 @@ static const struct snd_soc_dapm_widget msm_qdsp6_widgets[] = {
>> mmul7_mixer_controls, ARRAY_SIZE(mmul7_mixer_controls)),
>> SND_SOC_DAPM_MIXER("MultiMedia8 Mixer", SND_SOC_NOPM, 0, 0,
>> mmul8_mixer_controls, ARRAY_SIZE(mmul8_mixer_controls)),
>> -
>> };
>>
>> static const struct snd_soc_dapm_route intercon[] = {
>> @@ -1043,6 +1049,8 @@ static const struct snd_soc_dapm_route intercon[] = {
>> {"MM_UL6", NULL, "MultiMedia6 Mixer"},
>> {"MM_UL7", NULL, "MultiMedia7 Mixer"},
>> {"MM_UL8", NULL, "MultiMedia8 Mixer"},
>> +
>> + Q6ROUTING_RX_DAPM_ROUTE("USB Mixer", "USB_RX"),
>
> Put this below "Q6ROUTING_RX_DAPM_ROUTE("RX_CODEC_DMA_RX_7 Audio Mixer".
>
Sorry, missed that part. Fixed the naming and position of this.
Thanks
Wesley Cheng
Powered by blists - more mailing lists