Re: [PATCH v36 22/31] ASoC: qcom: qdsp6: Introduce USB AFE port to q6dsp

[Stephan Gerhold <stephan.gerhold@linaro.org>]

Hi Wesley,

On Mon, Mar 31, 2025 at 12:52:19PM -0700, Wesley Cheng wrote:
> On 3/26/2025 2:57 AM, Stephan Gerhold wrote:
> > On Tue, Mar 25, 2025 at 04:18:03PM -0700, Wesley Cheng wrote:
> > > On 3/25/2025 2:24 AM, Stephan Gerhold wrote:
> > > > On Tue, Mar 18, 2025 at 05:51:32PM -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@quicinc.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         |  32 +++-
> > > > >  6 files changed, 341 insertions(+), 3 deletions(-)
> > > > > 
> > > [...]
> > > > > diff --git a/sound/soc/qcom/qdsp6/q6routing.c b/sound/soc/qcom/qdsp6/q6routing.c
> > > > > index 90228699ba7d..b7439420b425 100644
> > > > > --- a/sound/soc/qcom/qdsp6/q6routing.c
> > > > > +++ b/sound/soc/qcom/qdsp6/q6routing.c
> > > > > @@ -435,6 +435,26 @@ static struct session_data *get_session_from_id(struct msm_routing_data *data,
> > > > >  	return NULL;
> > > > >  }
> > > > > +
> > > > > +static bool is_usb_routing_enabled(struct msm_routing_data *data)
> > > > > +{
> > > > > +	int i;
> > > > > +
> > > > > +	/*
> > > > > +	 * Loop through current sessions to see if there are active routes
> > > > > +	 * to the USB_RX backend DAI.  The USB offload routing is designed
> > > > > +	 * similarly to the non offload path.  If there are multiple PCM
> > > > > +	 * devices associated with the ASoC platform card, only one active
> > > > > +	 * path can be routed to the USB offloaded endpoint.
> > > > > +	 */
> > > > > +	for (i = 0; i < MAX_SESSIONS; i++) {
> > > > > +		if (data->sessions[i].port_id == USB_RX)
> > > > > +			return true;
> > > > > +	}
> > > > > +
> > > > > +	return false;
> > > > > +}
> > > > 
> > > > What is different about USB_RX compared to other output ports we have in
> > > > Q6AFE? Obviously, we can only play one stream on an output port. But
> > > > doesn't the ADSP mix streams together when you have multiple routes?
> > > > 
> > > 
> > > This patch will limit the USB_RX from being able to be mixed to multiple
> > > q6adm paths.
> > > 
> > > > Also, this doesn't actually check for *active* routes only. It just
> > > > looks if any other MultiMedia DAI is configured to output to USB_RX.
> > > > That doesn't mean they will ever be active at the same time.
> > > > 
> > > 
> > > Yes, the main reason being that that is the mechanism we use to populate
> > > the active offload path within the USB SND card mixer.
> > > 
> > > > I might for example want to have MultiMedia1 and MultiMedia2 both
> > > > configured to output to USB_RX. Let's assume MultiMedia1 is a normal PCM
> > > > DAI, MultiMedia2 is a compress offload DAI. When I want to playback
> > > > normal audio, I go through MultiMedia1, when I want to play compressed
> > > > audio, I go through MultiMedia2. Only one of them active at a time.
> > > > Why can't I set this up statically in the mixers?
> > > > 
> > > > If you confirm that it is really impossible to have multiple streams
> > > > mixed together to the USB_RX output in the ADSP, then this should be a
> > > > runtime check instead when starting the stream IMO.
> > > > 
> > > 
> > > We can have multiple streams being mixed together, but it will get
> > > confusing because it changes the definition that we had discussed about in
> > > the past about the overall design for the interaction w/ userspace.
> > > Although we (QC) only support a single USB audio device for offloading,
> > > there could be other situations where the audio DSP can support multiple
> > > devices.  The assumption is that each MM path is assigned to a USB device.
> > > 
> > 
> > Are you referring to the "USB Offload Playback Route PCM#*" mixers here?
> > They could just refer to first of the configured MM paths, if someone
> > decides to route multiple paths to the USB backend. Looking at
> > q6usb_update_offload_route(), I think the implementation does that
> > already.
> > 
> > I think it's fine that the userspace API for automatically "probing" the
> > PCM device supports only a single path to the USB backend. But if
> > someone wants to bypass the automatic probing and configure a more
> > advanced setup, do we need to forbid that?
> > 
> > Asked differently: what would happen if we remove this check here and
> > handle USB_RX like any other Q6AFE output port? Would anything break for
> > the userspace interface?
> > 
> 
> So I took a look at seeing how the Q6ADM/ASM interactions would work for
> the situation where if user tried to start both MM1/2 streams at the same
> time over the USB offload path.  In this scenario, we see that the Q6USB BE
> DAI operations, ie startup, hw_params, etc... gets called one time for the
> initial stream.  For example, if I start playback on MM1, then that
> triggers the USB BE DAI to be brought up.
> 
> When I start playback on MM2, since MM1 already called
> dpcm_be_dai_startup(), then be->dpcm[stream].users will be greater than
> zero.  This would cause the __soc_pcm_open() to be skipped for the USB BE
> DAI, so I wouldn't be able to check the runtime status at the Q6USB backend
> DAI.  However, we do track current streaming sessions done over Q6 ADM and
> it does save the AFE port associated to each COPP allocation, so I think its
> reasonable to see if there is already a COPP entry for the USB AFE port, to
> fail the open() call associated to the FE DAI.
> 

This sounds like a reasonable approach *if* we have to prevent multiple
MM DAIs from streaming to the USB AFE port at the same time.

It's still unclear to me why we have to introduce this limitation in the
first place. I think the questions from my previous email are still
open. Can you check them again?

Thanks,
Stephan