Linux Sound subsystem development
 help / color / mirror / Atom feed
From: Cezary Rojewski <cezary.rojewski@intel.com>
To: Takashi Iwai <tiwai@suse.de>
Cc: <broonie@kernel.org>, <tiwai@suse.com>, <perex@perex.cz>,
	<amadeuszx.slawinski@linux.intel.com>,
	<linux-sound@vger.kernel.org>, <gregkh@linuxfoundation.org>,
	<quic_wcheng@quicinc.com>, <mathias.nyman@linux.intel.com>
Subject: Re: [RFC 00/15] ALSA/ASoC: USB Audio Offload
Date: Thu, 17 Apr 2025 12:15:00 +0200	[thread overview]
Message-ID: <5f3c62e3-6993-4595-80fc-eb77c0c11f1d@intel.com> (raw)
In-Reply-To: <87sem9xuxs.wl-tiwai@suse.de>

On 2025-04-15 6:15 PM, Takashi Iwai wrote:
> On Fri, 11 Apr 2025 11:39:55 +0200,
> Cezary Rojewski wrote:
>>
>> On 2025-04-10 12:24 PM, Takashi Iwai wrote:
>>> On Wed, 09 Apr 2025 13:07:15 +0200,
>>> Cezary Rojewski wrote:

...

>>>> Apart from changes to sound/usb, the patchset contains exemplary
>>>> ASoC-based, offload-aware USB driver and a small sound card on the
>>>> avs-driver side to show how card/dai_link initialization looks like.
>>>>
>>>> In short, USB Audio Offload functionality lowers CPU usage when
>>>> streaming PCM over a USB Audio-Class device.  It has been first
>>>> introduced in xHCI 1.2 release and is described in section 7.9 [3].
>>>> Once hardware is prepared with hw_params(), all the USB endpoints
>>>> operations e.g.: start, stop, submitting URBs, are performed
>>>> internally, by the hardware and AudioDSP firmware.  Software driver
>>>> shall not intervene.
>>>>
>>>> Startup flow from:
>>>>
>>>> 	usb_pcm_open()
>>>> 	usb_pcm_hw_params()
>>>> 		snd_usb_endpoint_open()
>>>> 	usb_pcm_prepare()
>>>> 		usb_set_interface()
>>>> 		snd_usb_endpoint_start()
>>>> 	usb_pcm_trigger(cmd: START/STOP etc.)
>>>>
>>>> reduced to:
>>>>
>>>> 	usb_pcm_open()
>>>> 	usb_pcm_hw_params()
>>>> 		snd_usb_endpoint_open()
>>>> 	usb_pcm_prepare()
>>>> 		usb_set_interface()
>>>
>>> Hmm, how can it be?  The start of EP at prepare stage is done only
>>> conditionally for non-lowlatency or implicit-feedback mode, for
>>> example.
>>
>>
>> On Intel architecture the AudioDSP utilizes internal channel to
>> communicate with xHCI and perform all the transfer operations.  In
>> essence, once SET_INTERFACE TRB is done, the usb-driver is not
>> supposed to do anything.  The avs-driver (sound driver) would be the
>> trigger here - sends the start/stop/etc. requests to the AudioDSP
>> firmware, DSP does the rest.
>>
>> By trigger I mean SET_PIPELINE_STATE IPC and avs_path_xxx() which are
>> utilized for any transfer-type (sound/soc/intel/avs/pcm.c).
> 
> But does the DSP handles the different ways for the implicit feedback
> mode, the low-latency playback and else?  A specific mode like the
> implicit feedback mode is mandatory for many devices.  Similarly, the
> low-latency mode is essential for some application setups
> (e.g. pipewire or JACK prefers), while it's not always applicable
> depending on the PCM parameters (such as the free-wheeling mode).
> 
> AFAIK, the qcom offloading doesn't support those fully, hence the
> configuration must be dynamic for them.
The hw design is unfortunately not simple:

xHCI <> SIO <> ALH
^ USB side	^Audio DSP side

It's an internal channel between so-called Audio Sideband located on 
xHCI controller side, goes through Scalable-IO and links with Audio Link 
Hub that's acts as 'front-end' for DSP.  Every block has their own 
requirements and restrictions.

And thus the number of available 'streams' in such configuration is very 
limited - typically to just 6.  These are not even bi-direction streams 
but an internal map: IDs {1, 2} for outputs, IDs {3, 4, 5, 6} for 
inputs.  Due to such small number of available streams, there is 
pressure to filter USB devices.  In regard to endpoint types, only DATA 
and FEEDBACK are supported, IMPLICIT_FEEDBACK is not.  Such device 
should fail offload-candidate filtering and be serviced by snd_usb_audio 
driver instead.

The official xHCI spec and its section regarding Audio Sideband is 
tailored for "claim entire device for offload or ignore" approach:

- scan the USB device descriptors
- verify if it's an offload-candidate
- count number of DATA + FEEDBACK endpoints
- ask xHCI to _reserve_ Audio Sideband resources up-front. This means 
the resources are reserved long before sound-devices are visible from 
userspace, yet alone opened for streaming

- pcm->open/close() just ask xHCI to perform 
SET_ASSIGNMENT(reserved_resources_here) TRBs to prepare/close the 
internal channel for runtime operations, that's it

There is no intention to allow for switching the ownership between 
offload-aware and offload-unaware.  Either entire USB device and all its 
endpoints have Audio Sideband resources pre-allocated or no offload is 
present for the device at all.

...

>>>> The design goals:
>>>> 	- make ASoC first class citizen of sound/usb
>>>> 	- re-use code found in sound/usb, mimic HDAudio integration in ASoC:
>>>> 	  small sound/soc/codecs/hda.c driver leveraging power of entire
>>>> 	  sound/pci/hda/
>>>> 	- no shared control over a USB device, either snd_usb_audio or
>>>> 	  its ASoC equivalent takes control of the device
>>>>
>>>> To do that, major tasks are identified:
>>>>
>>>> a) On ASoC side 'struct snd_card' is part of 'struct snd_soc_card' and
>>>> is managed by the framework.  Similar situation with 'struct snd_pcm'
>>>> and rtd->pcm.  To keep the teardown path sane, drop card->private_free()
>>>> and pcm->private_free() usage.
>>>
>>> Well, this is a generic problem of ASoC framework.
>>> I believe this should be better handled in ASoC core side at first.
>>> e.g. the card object could be created at the very first step of the
>>> snd_soc_card creation, too (but without the actual slot assignment or
>>> device creation).
>>
>>
>> Well, in my opinion the card's tailing private context (extra size),
>> private_free() and all that comes with them in sound/core brings
>> unnecessary complexity to the ALSA framework.  devm_xxx() is
>> enough. Polluting snd_card and sound/core/init.c with driver-specifics
>> causes the teardown procedures on the ALSA framework side harder to
>> read/maintain.
> 
> Well, I'd say it depends, and pretty much a matter of taste.  The
> resources managed by the card free should be the last thing that is
> tied with the card object itself.  So, yes, it can be devm, but this
> wouldn't make things easier; the devm is merely a serialized release,
> after all.
> 
>>>> b) To initialize ASoC components/DAI properly, PCM capabilities should be
>>>> known up-front.  To do that, existing USB card probe() has to be split.
>>>>   From one-stage to two-stage process:
>>>>
>>>> 	- look ahead and parse usb_interface descriptors for PCM endpoints
>>>> 	  but do not create any PCMs (sound devices) yet
>>>> 	- create all PCMs based on obtained ->pcm_list and follow with
>>>> 	  MIDI/mixers/media
>>>>
>>>> Such approach allows to feed DAIs proper data even when a valid
>>>> sound-card pointer is not yet present - the initialization occurs before
>>>> snd_soc_bind_card() is called.
>>>    This one is another thing that is needed to adjust for ASoC
>>> framework.  But when snd_card object is available, this can be
>>> resolved automatically, too?  e.g. snd_pcm object or such can be
>>> created at that point.  The actual device registration is done anyway
>>> later via snd_device_register() call.
>>
>>
>> While I'm up for upgrading either framework in any area necessary to
>> have proper UAO support in ASoC, not sure whether it's a good idea to
>> make it a requirement for the feature.  This will enlarge the series -
>> well, guess it will be separate series (dependency) entirely.
> 
> The series contains lots of hackish API exposure, and I really would
> like to avoid that if possible.  In the case of HD-audio, the whole
> stuff was moved to its own bus at first for adapting to ASoC hd-audio
> ext implementation, but for USB-audio, I don't see the need for that
> yet.  And, if the concern is about the snd_card object lifecycle and
> ASoC binding, it can be improved in the lower level at first.

I tried hard to avoid hackish stuff, perhaps not hard enough :)  Would 
it be possible to list the areas which you believe need to be look at? 
This is frankly the feedback which I'm most interested in.  Mathias gave 
me a number of these, e.g.: do not access udev->slot_id in sound/, avoid 
manipulating hcd/controller in sound/ and such.

In regard to the HDAudio point, I see clear benefits by having HDAudio 
and USB aligned in the approach on ASoC side. It's a path that's known, 
works and is well tested.

In regard to the snd_soc_card vs snd_card, in my opinion the refactor of 
card initialization is a large task.  In the past I did similar change 
for the snd_soc_component [1] but the card is an entirely different 
beast.  That's why I'm suggesting incremental approach - update ASoC 
initialization as a next step.


[1]: 
https://lore.kernel.org/all/20200731144146.6678-1-cezary.rojewski@intel.com/

Kind regards,
Czarek

  reply	other threads:[~2025-04-17 10:15 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-09 11:07 [RFC 00/15] ALSA/ASoC: USB Audio Offload Cezary Rojewski
2025-04-09 11:07 ` [RFC 01/15] ALSA: usb: Move media-filters to the media code Cezary Rojewski
2025-04-09 11:07 ` [RFC 02/15] ALSA: usb: Drop private_free() usage for card and pcms Cezary Rojewski
2025-04-09 11:07 ` [RFC 03/15] ALSA: usb: Relocate the usbaudio header file Cezary Rojewski
2025-04-09 11:07 ` [RFC 04/15] ALSA: usb: Implement two-stage quirk applying mechanism Cezary Rojewski
2025-04-09 11:07 ` [RFC 05/15] ALSA: usb: Implement two-stage stream creation mechanism Cezary Rojewski
2025-04-09 11:07 ` [RFC 06/15] ALSA: usb: Implement two-stage chip probing mechanism Cezary Rojewski
2025-04-09 11:07 ` [RFC 07/15] ALSA: usb: Switch to the two-stage chip probing Cezary Rojewski
2025-04-09 11:07 ` [RFC 08/15] ALSA: usb: Switch to the two-stage stream creation Cezary Rojewski
2025-04-09 11:07 ` [RFC 09/15] ALSA: usb: Switch to the two-stage quirk applying Cezary Rojewski
2025-04-09 11:07 ` [RFC 10/15] ALSA: usb: Export PCM operations Cezary Rojewski
2025-04-09 11:07 ` [RFC 11/15] ALSA: usb: Export usb_interface driver operations Cezary Rojewski
2025-04-09 11:07 ` [RFC 12/15] ALSA: usb: Export card-naming procedure Cezary Rojewski
2025-04-09 11:07 ` [RFC 13/15] ALSA: usb: Add getters to obtain endpoint information Cezary Rojewski
2025-04-09 11:07 ` [RFC 14/15] ASoC: codecs: Add USB-Audio driver Cezary Rojewski
2025-04-09 11:07 ` [RFC 15/15] ASoC: Intel: avs: Add USB machine board Cezary Rojewski
2025-04-09 12:10 ` [RFC 00/15] ALSA/ASoC: USB Audio Offload Greg KH
2025-04-09 13:06   ` Cezary Rojewski
2025-04-10 10:10     ` Takashi Iwai
2025-04-10 10:24 ` Takashi Iwai
2025-04-11  9:39   ` Cezary Rojewski
2025-04-15 16:15     ` Takashi Iwai
2025-04-17 10:15       ` Cezary Rojewski [this message]
2025-04-22 11:28         ` Pierre-Louis Bossart
2025-04-22 14:15           ` Cezary Rojewski
2025-04-25 16:53             ` Pierre-Louis Bossart
2025-04-11 14:04 ` Greg KH
2025-04-11 16:51   ` Cezary Rojewski

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=5f3c62e3-6993-4595-80fc-eb77c0c11f1d@intel.com \
    --to=cezary.rojewski@intel.com \
    --cc=amadeuszx.slawinski@linux.intel.com \
    --cc=broonie@kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-sound@vger.kernel.org \
    --cc=mathias.nyman@linux.intel.com \
    --cc=perex@perex.cz \
    --cc=quic_wcheng@quicinc.com \
    --cc=tiwai@suse.com \
    --cc=tiwai@suse.de \
    /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: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox