Re: [PATCH v9 09/34] ASoC: qcom: qdsp6: Introduce USB AFE port to q6dsp

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Pierre,

On 10/18/2023 6:47 AM, Pierre-Louis Bossart wrote:

Specifically, the QC ADSP can support all potential endpoints that are
exposed by the audio data interface.  This includes, feedback endpoints
(both implicit and explicit) as well as the isochronous (data)
endpoints.

implicit feedback means support for capture. This is confusing...


I mean, a USB device can expose a capture path, but as of now, we won't
enable the offloading to the audio DSP for it.  However, if we're
executing playback, and device does support implicit feedback, we will
pass that along to the audio DSP to utilize.

Not following. Implicit feedback means a capture stream *SHALL* be
started. Are you saying this capture stream is hidden and handled at the
DSP level only? If yes, what prevents you from exposing the capture
stream to userspace as well?

I must be missing something.


My understanding is that with implicit feedback endpoints, it allows for another data endpoint in the opposite direction to be utilized as a feedback endpoint (versus having to expose another EP, such as in the case of explicit feedback). For example, if we are enabling the playback path (and the device does have a capture data ep) then the data ep used for the capture path can be used.

USB2.0 spec, section 5.12.4.3 (Implicit Feedback)
"
Two cases can arise:
• One or more asynchronous sink endpoints are accompanied by an asynchronous source endpoint. The data rate on the source endpoint can be used as implicit feedback information to adjust the data rate on
the sink endpoint(s).
• One or more adaptive source endpoints are accompanied by an adaptive sink endpoint. The source endpoint can adjust its data rate based on the data rate received by the sink endpoint.
"

The DSP will get this as part of the USB sync endpoint information which it will use to enable this EP.

   +static const struct snd_soc_dai_ops q6usb_ops = {
+    .probe        = msm_dai_q6_dai_probe,
+    .prepare    = q6afe_dai_prepare,
+    .hw_params    = q6usb_hw_params,

this is rather confusing with two different layers used for hw_params
and prepare? Additional comments or explanations wouldn't hurt.


I thought this was how the ASoC design was.  Each DAI defined for a
particular path has it own set of callbacks implemented to bring up any
required resources for that entity.  So in this case, it initializes the
"cpu" DAI, which is the main component that handles communication with
the audio DSP.

Usually prepare and hw_params rely on the type of DAI callbacks, but
here you are mixing "q6afe" and "q6usb" which are shown in your Patch0
diagram as "cpu" and "codec" dais respectively. I don't think it's
correct to tie the two, it's a clear layering violation IMHO. The codec
dai .prepare should not invoke something that modifies the state of the
CPU dai, which should have its own .prepare callback.

OK, I think I know where the misunderstanding is. The API callback for hw_params() that is being registered here exists in q6afe, but with the q6usb prefix. I will fix that in the next rev.

Thanks
Wesley Cheng



[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux