The Q6USB backend can carry information about the available USB SND cards
and PCM devices 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. Separate this into a dedicated API, so the USB
backend can set the dev_token accordingly. The audio DSP does not utilize
this information until the AFE port start command is sent, which is done
during the PCM prepare phase.
Signed-off-by: Wesley Cheng <[email protected]>
---
sound/soc/qcom/qdsp6/q6afe.c | 49 +++++++++++++++++++++++++-----------
sound/soc/qcom/qdsp6/q6afe.h | 1 +
2 files changed, 36 insertions(+), 14 deletions(-)
diff --git a/sound/soc/qcom/qdsp6/q6afe.c b/sound/soc/qcom/qdsp6/q6afe.c
index 72c4e6fe20c4..f09a756246f8 100644
--- a/sound/soc/qcom/qdsp6/q6afe.c
+++ b/sound/soc/qcom/qdsp6/q6afe.c
@@ -1394,10 +1394,42 @@ void q6afe_tdm_port_prepare(struct q6afe_port *port,
}
EXPORT_SYMBOL_GPL(q6afe_tdm_port_prepare);
-static int afe_port_send_usb_dev_param(struct q6afe_port *port, struct q6afe_usb_cfg *cfg)
+/**
+ * afe_port_send_usb_dev_param() - Send USB dev token
+ *
+ * @port: Instance of afe port
+ * @cardidx: USB SND card index to reference
+ * @pcmidx: USB SND PCM device index to reference
+ *
+ * The USB dev token carries information about which USB SND card instance and
+ * PCM device to execute the offload on. This information is carried through
+ * to the stream enable QMI request, which is handled by the offload class
+ * driver. The information is parsed to determine which USB device to query
+ * the required resources for.
+ */
+int afe_port_send_usb_dev_param(struct q6afe_port *port, int cardidx, int pcmidx)
{
- union afe_port_config *pcfg = &port->port_cfg;
struct afe_param_id_usb_audio_dev_params usb_dev;
+ int ret;
+
+ memset(&usb_dev, 0, sizeof(usb_dev));
+
+ usb_dev.cfg_minor_version = AFE_API_MINOR_VERSION_USB_AUDIO_CONFIG;
+ usb_dev.dev_token = (cardidx << 16) | (pcmidx << 8);
+ ret = q6afe_port_set_param_v2(port, &usb_dev,
+ AFE_PARAM_ID_USB_AUDIO_DEV_PARAMS,
+ AFE_MODULE_AUDIO_DEV_INTERFACE, sizeof(usb_dev));
+ if (ret)
+ dev_err(port->afe->dev, "%s: AFE device param cmd failed %d\n",
+ __func__, ret);
+
+ return ret;
+}
+EXPORT_SYMBOL_GPL(afe_port_send_usb_dev_param);
+
+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 = 0;
@@ -1408,20 +1440,9 @@ static int afe_port_send_usb_dev_param(struct q6afe_port *port, struct q6afe_usb
goto exit;
}
- memset(&usb_dev, 0, sizeof(usb_dev));
memset(&lpcm_fmt, 0, sizeof(lpcm_fmt));
memset(&svc_int, 0, sizeof(svc_int));
- usb_dev.cfg_minor_version = AFE_API_MINOR_VERSION_USB_AUDIO_CONFIG;
- ret = q6afe_port_set_param_v2(port, &usb_dev,
- AFE_PARAM_ID_USB_AUDIO_DEV_PARAMS,
- AFE_MODULE_AUDIO_DEV_INTERFACE, sizeof(usb_dev));
- if (ret) {
- dev_err(port->afe->dev, "%s: AFE device param cmd failed %d\n",
- __func__, ret);
- goto exit;
- }
-
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,
@@ -1463,7 +1484,7 @@ void q6afe_usb_port_prepare(struct q6afe_port *port,
pcfg->usb_cfg.num_channels = cfg->num_channels;
pcfg->usb_cfg.bit_width = cfg->bit_width;
- afe_port_send_usb_dev_param(port, cfg);
+ afe_port_send_usb_params(port, cfg);
}
EXPORT_SYMBOL_GPL(q6afe_usb_port_prepare);
diff --git a/sound/soc/qcom/qdsp6/q6afe.h b/sound/soc/qcom/qdsp6/q6afe.h
index ef47b4ae9e27..2ce5ba9dba69 100644
--- a/sound/soc/qcom/qdsp6/q6afe.h
+++ b/sound/soc/qcom/qdsp6/q6afe.h
@@ -263,6 +263,7 @@ void q6afe_tdm_port_prepare(struct q6afe_port *port, struct q6afe_tdm_cfg *cfg);
void q6afe_cdc_dma_port_prepare(struct q6afe_port *port,
struct q6afe_cdc_dma_cfg *cfg);
+int afe_port_send_usb_dev_param(struct q6afe_port *port, int cardidx, int pcmidx);
int q6afe_port_set_sysclk(struct q6afe_port *port, int clk_id,
int clk_src, int clk_root,
unsigned int freq, int dir);
On 10/17/23 15:01, Wesley Cheng wrote:
> The Q6USB backend can carry information about the available USB SND cards
> and PCM devices 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. Separate this into a dedicated API, so the USB
> backend can set the dev_token accordingly. The audio DSP does not utilize
> this information until the AFE port start command is sent, which is done
> during the PCM prepare phase.
>
> Signed-off-by: Wesley Cheng <[email protected]>
> ---
> sound/soc/qcom/qdsp6/q6afe.c | 49 +++++++++++++++++++++++++-----------
> sound/soc/qcom/qdsp6/q6afe.h | 1 +
> 2 files changed, 36 insertions(+), 14 deletions(-)
>
> diff --git a/sound/soc/qcom/qdsp6/q6afe.c b/sound/soc/qcom/qdsp6/q6afe.c
> index 72c4e6fe20c4..f09a756246f8 100644
> --- a/sound/soc/qcom/qdsp6/q6afe.c
> +++ b/sound/soc/qcom/qdsp6/q6afe.c
> @@ -1394,10 +1394,42 @@ void q6afe_tdm_port_prepare(struct q6afe_port *port,
> }
> EXPORT_SYMBOL_GPL(q6afe_tdm_port_prepare);
>
> -static int afe_port_send_usb_dev_param(struct q6afe_port *port, struct q6afe_usb_cfg *cfg)
> +/**
> + * afe_port_send_usb_dev_param() - Send USB dev token
> + *
> + * @port: Instance of afe port
> + * @cardidx: USB SND card index to reference
> + * @pcmidx: USB SND PCM device index to reference
> + *
> + * The USB dev token carries information about which USB SND card instance and
> + * PCM device to execute the offload on. This information is carried through
> + * to the stream enable QMI request, which is handled by the offload class
> + * driver. The information is parsed to determine which USB device to query
> + * the required resources for.
> + */
> +int afe_port_send_usb_dev_param(struct q6afe_port *port, int cardidx, int pcmidx)
> {
> - union afe_port_config *pcfg = &port->port_cfg;
> struct afe_param_id_usb_audio_dev_params usb_dev;
> + int ret;
> +
> + memset(&usb_dev, 0, sizeof(usb_dev));
> +
> + usb_dev.cfg_minor_version = AFE_API_MINOR_VERSION_USB_AUDIO_CONFIG;
> + usb_dev.dev_token = (cardidx << 16) | (pcmidx << 8);
> + ret = q6afe_port_set_param_v2(port, &usb_dev,
> + AFE_PARAM_ID_USB_AUDIO_DEV_PARAMS,
> + AFE_MODULE_AUDIO_DEV_INTERFACE, sizeof(usb_dev));
> + if (ret)
> + dev_err(port->afe->dev, "%s: AFE device param cmd failed %d\n",
> + __func__, ret);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(afe_port_send_usb_dev_param);
> +
> +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 = 0;
> @@ -1408,20 +1440,9 @@ static int afe_port_send_usb_dev_param(struct q6afe_port *port, struct q6afe_usb
> goto exit;
> }
>
> - memset(&usb_dev, 0, sizeof(usb_dev));
> memset(&lpcm_fmt, 0, sizeof(lpcm_fmt));
> memset(&svc_int, 0, sizeof(svc_int));
>
> - usb_dev.cfg_minor_version = AFE_API_MINOR_VERSION_USB_AUDIO_CONFIG;
> - ret = q6afe_port_set_param_v2(port, &usb_dev,
> - AFE_PARAM_ID_USB_AUDIO_DEV_PARAMS,
> - AFE_MODULE_AUDIO_DEV_INTERFACE, sizeof(usb_dev));
> - if (ret) {
> - dev_err(port->afe->dev, "%s: AFE device param cmd failed %d\n",
> - __func__, ret);
> - goto exit;
> - }
> -
this feels like a questionable patch split. Why not introduce the new
helper earlier and avoid adding code then modifying the same code?
> 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,
> @@ -1463,7 +1484,7 @@ void q6afe_usb_port_prepare(struct q6afe_port *port,
> pcfg->usb_cfg.num_channels = cfg->num_channels;
> pcfg->usb_cfg.bit_width = cfg->bit_width;
>
> - afe_port_send_usb_dev_param(port, cfg);
> + afe_port_send_usb_params(port, cfg);
> }
> EXPORT_SYMBOL_GPL(q6afe_usb_port_prepare);
>
> diff --git a/sound/soc/qcom/qdsp6/q6afe.h b/sound/soc/qcom/qdsp6/q6afe.h
> index ef47b4ae9e27..2ce5ba9dba69 100644
> --- a/sound/soc/qcom/qdsp6/q6afe.h
> +++ b/sound/soc/qcom/qdsp6/q6afe.h
> @@ -263,6 +263,7 @@ void q6afe_tdm_port_prepare(struct q6afe_port *port, struct q6afe_tdm_cfg *cfg);
> void q6afe_cdc_dma_port_prepare(struct q6afe_port *port,
> struct q6afe_cdc_dma_cfg *cfg);
>
> +int afe_port_send_usb_dev_param(struct q6afe_port *port, int cardidx, int pcmidx);
> int q6afe_port_set_sysclk(struct q6afe_port *port, int clk_id,
> int clk_src, int clk_root,
> unsigned int freq, int dir);
Hi Pierre,
On 10/17/2023 3:39 PM, Pierre-Louis Bossart wrote:
>
>
> On 10/17/23 15:01, Wesley Cheng wrote:
>> The Q6USB backend can carry information about the available USB SND cards
>> and PCM devices 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. Separate this into a dedicated API, so the USB
>> backend can set the dev_token accordingly. The audio DSP does not utilize
>> this information until the AFE port start command is sent, which is done
>> during the PCM prepare phase.
>>
>> Signed-off-by: Wesley Cheng <[email protected]>
>> ---
>> sound/soc/qcom/qdsp6/q6afe.c | 49 +++++++++++++++++++++++++-----------
>> sound/soc/qcom/qdsp6/q6afe.h | 1 +
>> 2 files changed, 36 insertions(+), 14 deletions(-)
>>
>> diff --git a/sound/soc/qcom/qdsp6/q6afe.c b/sound/soc/qcom/qdsp6/q6afe.c
>> index 72c4e6fe20c4..f09a756246f8 100644
>> --- a/sound/soc/qcom/qdsp6/q6afe.c
>> +++ b/sound/soc/qcom/qdsp6/q6afe.c
>> @@ -1394,10 +1394,42 @@ void q6afe_tdm_port_prepare(struct q6afe_port *port,
>> }
>> EXPORT_SYMBOL_GPL(q6afe_tdm_port_prepare);
>>
>> -static int afe_port_send_usb_dev_param(struct q6afe_port *port, struct q6afe_usb_cfg *cfg)
>> +/**
>> + * afe_port_send_usb_dev_param() - Send USB dev token
>> + *
>> + * @port: Instance of afe port
>> + * @cardidx: USB SND card index to reference
>> + * @pcmidx: USB SND PCM device index to reference
>> + *
>> + * The USB dev token carries information about which USB SND card instance and
>> + * PCM device to execute the offload on. This information is carried through
>> + * to the stream enable QMI request, which is handled by the offload class
>> + * driver. The information is parsed to determine which USB device to query
>> + * the required resources for.
>> + */
>> +int afe_port_send_usb_dev_param(struct q6afe_port *port, int cardidx, int pcmidx)
>> {
>> - union afe_port_config *pcfg = &port->port_cfg;
>> struct afe_param_id_usb_audio_dev_params usb_dev;
>> + int ret;
>> +
>> + memset(&usb_dev, 0, sizeof(usb_dev));
>> +
>> + usb_dev.cfg_minor_version = AFE_API_MINOR_VERSION_USB_AUDIO_CONFIG;
>> + usb_dev.dev_token = (cardidx << 16) | (pcmidx << 8);
>> + ret = q6afe_port_set_param_v2(port, &usb_dev,
>> + AFE_PARAM_ID_USB_AUDIO_DEV_PARAMS,
>> + AFE_MODULE_AUDIO_DEV_INTERFACE, sizeof(usb_dev));
>> + if (ret)
>> + dev_err(port->afe->dev, "%s: AFE device param cmd failed %d\n",
>> + __func__, ret);
>> +
>> + return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(afe_port_send_usb_dev_param);
>> +
>> +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 = 0;
>> @@ -1408,20 +1440,9 @@ static int afe_port_send_usb_dev_param(struct q6afe_port *port, struct q6afe_usb
>> goto exit;
>> }
>>
>> - memset(&usb_dev, 0, sizeof(usb_dev));
>> memset(&lpcm_fmt, 0, sizeof(lpcm_fmt));
>> memset(&svc_int, 0, sizeof(svc_int));
>>
>> - usb_dev.cfg_minor_version = AFE_API_MINOR_VERSION_USB_AUDIO_CONFIG;
>> - ret = q6afe_port_set_param_v2(port, &usb_dev,
>> - AFE_PARAM_ID_USB_AUDIO_DEV_PARAMS,
>> - AFE_MODULE_AUDIO_DEV_INTERFACE, sizeof(usb_dev));
>> - if (ret) {
>> - dev_err(port->afe->dev, "%s: AFE device param cmd failed %d\n",
>> - __func__, ret);
>> - goto exit;
>> - }
>> -
>
> this feels like a questionable patch split. Why not introduce the new
> helper earlier and avoid adding code then modifying the same code?
>
Let me see if I can squash this with the change that adds the USB AFE port.
Thanks
Wesley Cheng