Message-ID: <ca1e1063-e1bd-4e03-a7cd-91985e9954e9@linux.intel.com>
Date: Thu, 13 Jun 2024 09:46:02 +0200
From: Amadeusz Sławiński
 <amadeuszx.slawinski@...ux.intel.com>
To: Wesley Cheng <quic_wcheng@...cinc.com>, srinivas.kandagatla@...aro.org,
 mathias.nyman@...el.com, perex@...ex.cz, conor+dt@...nel.org,
 corbet@....net, broonie@...nel.org, lgirdwood@...il.com, krzk+dt@...nel.org,
 Thinh.Nguyen@...opsys.com, bgoswami@...cinc.com, tiwai@...e.com,
 robh@...nel.org, gregkh@...uxfoundation.org
Cc: linux-kernel@...r.kernel.org, devicetree@...r.kernel.org,
 linux-sound@...r.kernel.org, linux-usb@...r.kernel.org,
 linux-arm-msm@...r.kernel.org, linux-doc@...r.kernel.org,
 alsa-devel@...a-project.org
Subject: Re: [PATCH v23 32/32] ASoC: doc: Add documentation for SOC USB

On 6/12/2024 9:28 PM, Wesley Cheng wrote:
> Hi Amadeusz,
> 
> On 6/12/2024 7:47 AM, Amadeusz Sławiński wrote:
>> On 6/11/2024 1:58 AM, Wesley Cheng wrote:
>>
>> (...)
>>
>>> +In the case where the USB offload driver is unbounded, while USB SND is
>>
>> unbounded -> unbound
>>
>> (...)
>>
>>> +SOC USB and USB Sound Kcontrols
>>> +===============================
>>> +Details
>>> +-------
>>> +SOC USB and USB sound expose a set of SND kcontrols for applications 
>>> to select
>>> +and fetch the current offloading status for the ASoC platform sound 
>>> card. Kcontrols
>>> +are split between two layers:
>>> +
>>> +    - USB sound - Notifies the sound card number for the ASoC 
>>> platform sound
>>> +      card that it is registered to for supporting audio offload.
>>> +
>>> +    - SOC USB - Maintains the current status of the offload path, 
>>> and device
>>> +      (USB sound card and PCM device) information.  This would be 
>>> the main
>>> +      card that applications can read to determine offloading 
>>> capabilities.
>>> +
>>> +Implementation
>>> +--------------
>>> +
>>> +**Example:**
>>> +
>>> +  **Sound Cards**:
>>> +
>>> +    ::
>>> +
>>> +      0 [SM8250MTPWCD938]: sm8250 - SM8250-MTP-WCD9380-WSA8810-VA-D
>>> +                     SM8250-MTP-WCD9380-WSA8810-VA-DMIC
>>> +      1 [C320M          ]: USB-Audio - Plantronics C320-M
>>> +                     Plantronics Plantronics C320-M at 
>>> usb-xhci-hcd.1.auto-1, full speed
>>> +
>>> +
>>> +  **Platform Sound Card** - card#0:
>>> +
>>> +    ::
>>> +
>>> +      USB Offload Playback Route Card Select  1 (range -1->32)
>>> +      USB Offload Playback Route PCM Select   0 (range -1->255)
>>> +      USB Offload Playback Route Card Status  -1 (range -1->32)
>>> +      USB Offload Playback Route PCM Status   -1 (range -1->255)
>>> +
>>> +
>>> +  **USB Sound Card** - card#1:
>>> +
>>> +    ::
>>> +
>>> +      USB Offload Playback Capable Card         0 (range -1->32)
>>> +
>>> +
>>> +The platform sound card(card#0) kcontrols are created as part of 
>>> adding the SOC
>>> +USB device using **snd_soc_usb_add_port()**.  The following 
>>> kcontrols are defined
>>> +as:
>>> +
>>> +  - ``USB Offload Playback Route Card Status`` **(R)**: USB sound 
>>> card device index
>>> +    that defines which USB SND resources are currently offloaded.  
>>> If -1 is seen, it
>>> +    signifies that offload is not active.
>>> +  - ``USB Offload Playback Route PCM Status`` **(R)**: USB PCM 
>>> device index
>>> +    that defines which USB SND resources are currently offloaded.  
>>> If -1 is seen, it
>>> +    signifies that offload is not active.
>>> +  - ``USB Offload Playback Route Card Select`` **(R/W)**: USB sound 
>>> card index which
>>> +    selects the USB device to initiate offloading on.  If no value 
>>> is written to the
>>> +    kcontrol, then the last USB device discovered card index will be 
>>> chosen.
>>
>> I see only one kcontrol, what if hardware is capable of offloading on 
>> more cards, is it possible to do offloading on more than one device?
>>
>>> +  - ``USB Offload Playback Route PCM Select`` **(R/W)**: USB PCM 
>>> index which selects
>>> +    the USB device to initiate offloading on.  If no value is 
>>> written to the
>>> +    kcontrol, then the last USB device discovered PCM zero index 
>>> will be chosen.
>>> +
>>> +The USB sound card(card#1) kcontrols are created as USB audio 
>>> devices are plugged
>>> +into the physical USB port and enumerated.  The kcontrols are 
>>> defined as:
>>> +
>>> +  - ``USB Offload Playback Capable Card`` **(R)**: Provides the 
>>> sound card
>>> +    number/index that supports USB offloading.  Further/follow up 
>>> queries about
>>> +    the current offload state can be handled by reading the offload 
>>> status
>>> +    kcontrol exposed by the platform card.
>>> +
>>
>>
>> Why do we need to some magic between cards? I feel like whole kcontrol 
>> thing is overengineered a bit - I'm not sure I understand the need to 
>> do linking between cards. It would feel a lot simpler if USB card 
>> exposed one "USB Offload" kcontrol on USB card if USB controller 
>> supports offloading and allowed to set it to true/false to allow user 
>> to choose if they want to do offloading on device.
>>
>> (...)
> 
> Based on feedback from Pierre, what I understood is that for some 
> applications, there won't be an order on which sound card is 
> queried/opened first.
> 

Yes if you have multiple cards, they are probed in random order.

> So the end use case example given was if an application opened the USB 
> sound card first, it can see if there is an offload path available.  If 
> there is then it can enable the offload path on the corresponding card 
> if desired.
> 

This still doesn't explain why you need to link cards using controls. 
What would not work with simple "Enable Offload" with true/false values 
on USB card that works while you do have above routing controls?

>>> +Mixer Examples
>>> +--------------
>>> +
>>> +    ::
>>> +
>>> +      tinymix -D 0 set 'USB Offload Playback Route Card Select' 2
>>> +      tinymix -D 0 set 'USB Offload Playback Route PCM Select' 0
>>> +
>>> +
>>> +    ::
>>> +
>>> +      tinymix -D 0 get 'USB Offload Playback Route Card Select'
>>> +      --> 2 (range -1->32)
>>> +      tinymix -D 0 get 'USB Offload Playback Route PCM Select'
>>> +      --> 0 (range -1->255)
>>> +
>>> +    ::
>>> +
>>> +      tinymix -D 0 get 'USB Offload Playback Route Card Status'
>>> +      --> 2 (range -1->32)   [OFFLD active]
>>> +      --> -1 (range -1->32) [OFFLD idle]
>>> +      tinymix -D 0 get 'USB Offload Playback Route PCM Status'
>>> +      --> 0 (range -1->255)   [OFFLD active]
>>> +      --> -1 (range -1->255) [OFFLD idle]
>>> +
>>> +    ::
>>> +
>>> +      tinymix -D 1 get 'USB Offload Playback Capable Card'
>>> +      --> 0 (range -1->32)
>>>
>>
>> Yes, looking at examples again, I'm still not sure I understand. There 
>> are two cards and you do linking between them, this feels broken by 
>> design. From my point of view USB Offload should be property of USB 
>> card and not involve any other card in a system.
>>
> 
> Main benefit to having two cards (keeping one for USB SND and another 
> for the ASoC platform sound card) is that current applications won't 
> break.  The behavior is the same, in that if something opens the USB 
> sound card, it will go through the same non-offloaded path.  During 
> initial reviews, I think this was a big point where folks wanted the USB 
> PCM path to still be an option.
> 

I'm not against having two cards, in fact I hope that USB card looks and 
behaves the same as before this patch set, with only difference being 
controls for enabling offload.

> If applications want to add the offload capabilities to its environment, 
> they can enable it as an additional feature.

That sounds fine to me.

Powered by blists - more mailing lists