devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Amadeusz Sławiński" <amadeuszx.slawinski@linux.intel.com>
To: Wesley Cheng <quic_wcheng@quicinc.com>,
	srinivas.kandagatla@linaro.org, mathias.nyman@intel.com,
	perex@perex.cz, conor+dt@kernel.org, corbet@lwn.net,
	broonie@kernel.org, lgirdwood@gmail.com, krzk+dt@kernel.org,
	Thinh.Nguyen@synopsys.com, bgoswami@quicinc.com, tiwai@suse.com,
	robh@kernel.org, gregkh@linuxfoundation.org
Cc: linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	linux-sound@vger.kernel.org, linux-usb@vger.kernel.org,
	linux-arm-msm@vger.kernel.org, linux-doc@vger.kernel.org,
	alsa-devel@alsa-project.org
Subject: Re: [PATCH v23 32/32] ASoC: doc: Add documentation for SOC USB
Date: Wed, 19 Jun 2024 09:52:32 +0200	[thread overview]
Message-ID: <510468c7-b181-48d0-bf2d-3e478b2f2aca@linux.intel.com> (raw)
In-Reply-To: <eb6370ea-47a0-3659-3c10-cb7f95e3e520@quicinc.com>

On 6/18/2024 10:52 PM, Wesley Cheng wrote:
> Hi Amadeusz,
> 
> On 6/18/2024 4:42 AM, Amadeusz Sławiński wrote:
>> On 6/17/2024 7:02 PM, Wesley Cheng wrote:
>>> Hi Amadeusz,
>>>
>>> On 6/13/2024 12:46 AM, Amadeusz Sławiński wrote:
>>>> 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?
>>>>
>>>
>>> Sorry for the late response.
>>>
>>> I think either way, even with the "Enable Offload" kcontrol in USB 
>>> SND, we'd need a way to link these cards, because if you have 
>>> multiple USB audio devices connected, and say... your offload 
>>> mechanism only supports one stream.  Then I assume we'd still need to 
>>> way to determine if that stream can be enabled for that USB SND 
>>> device or not.
>>>
>>> Since the USB SND isn't really the entity maintaining the offload 
>>> path, I went with the decision to add that route selection to the 
>>> ASoC platform card. It would have access to all the parameters 
>>> supported by the audio DSP.
>>>
>>
>> Problem with card selection is that it will most likely work in pretty 
>> random way during reboots and similar scenarios.
>>
>> Taking from your example:
>>      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)
>>
>> This tells that hw:1,0 will be offloaded USB card. What happens if 
>> after reboot the USB card and offload card change places, the control 
>> will be pointing at its owner... Another scenario to consider is that 
>> user attaches two USB cards and only first one does offload. Now what 
>> happens when they enumerate in different order after reboot (swapping 
>> places)? Taking into the account that most systems restore previous 
>> values of controls in some way - this will point at wrong card.
> 
> That sounds like a problem that would exist with current USB SND 
> implementation too?  Removing the offloading perspective, how does the 
> system ensure that the previous setting stays persistent?  For example, 
> as you mentioned, depending on which USB device enumerates first, the 
> sound card may be different so cards will be switched.
> 

It works because there is no control pointing at other card. My main 
problem is with controls which have card and pcm id of other card in it.

> I think I mentioned this previously in another discussion, but I think 
> the idea was that with the
> USB Offload Playback Capable Card
> 
> kcontrol, would allow the system to at least know there is an offload 
> capable path pointing to the ASoC platform card, and fetch more detailed 
> information about which device is selected for offloading, etc...
> 

This works only in your design, where USB Offload is backed by card, 
what happens if it is backed by something else?

>>
>> In my opinion Offload capability should be the capability of the 
>> endpoint - in this case USB card (even if in the background it needs 
>> to talk to some other device) and it should be exposed as such. 
>> Currently you are mixing capabilities of your audio card with 
>> capabilities of USB card.
>>
>> And adding more controls will not make it easy to use from end user 
>> perspective. Most users will most likely want for the devices to 
>> perform offload automatically if possible to save power and just have 
>> control to disable it in case they want to test if it works better 
>> without it in case of some problems.
> 
> I agree with you that we need to keep the controls at a minimum, but I 
> think what I have in place is fairly reasonable.  If we switch to having 
> the USB SND controlling things, we'd save maybe one control?  I think 
> keeping the offload status controls are still fairly valuable in both 
> scenarios, as userspace may need to verify which USB SND card is being 
> offloaded.
> 

It should be able to tell which one is being offloaded by examining 
which USB card has Offload control set to true.

I would assume that USB cards that cannot perform Offload have no 
control at all, as it is unneeded. And ones that can do, have Offload 
control. And ones actively being Offloaded have it set to true, 
otherwise to false.

End user has no need to know where it is offloaded. I'm not HW person, 
but I would assume that it is even unlikely that someone will design HW, 
where it is possible to Offload one endpoint to two different places, as 
this complicates things a lot, but if it were possible, from design 
perspective it would make a lot more sense to set it in Offloaded USB 
card settings, instead of some seemingly unrelated controller card 
device. And that is assuming that all solutions use some other card 
device to perform Offload.

>>
>> Additional question what happens if you want to offload two usb cards, 
>> currently the above set of controls allows you to only point at one 
>> card, will you be adding additional set of above controls dynamically 
>> for each USB card attached?
>>
> 
> It would depend on the number of offload streams that folks may be 
> supporting on their platform.  In our case we only have one available 
> stream, so applications would need to switch between the two devices 
> using the card/pcm selector.
> 
> In this case, there will be only one set of controls to select the 
> card/pcm device.  As of now (I think I'll change to to add another 
> separate set of controls per stream) if you did support multiple 
> streams, then the current card/PCM device selector would take in 
> multiple arugments. (ie for two streams the kcontrol can take in two 
> values)
> 

Then it is implementation detail of your device, and it should be 
implemented as controls in your device instead of as part of generic API.

Thanks,
Amadeusz

  reply	other threads:[~2024-06-19  7:52 UTC|newest]

Thread overview: 60+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-10 23:57 [PATCH v23 00/32] Introduce QC USB SND audio offloading support Wesley Cheng
2024-06-10 23:57 ` [PATCH v23 01/32] ASoC: Add SOC USB APIs for adding an USB backend Wesley Cheng
2024-06-10 23:57 ` [PATCH v23 02/32] ASoC: dt-bindings: qcom,q6dsp-lpass-ports: Add USB_RX port Wesley Cheng
2024-06-10 23:57 ` [PATCH v23 03/32] ASoC: qcom: qdsp6: Introduce USB AFE port to q6dsp Wesley Cheng
2024-06-10 23:57 ` [PATCH v23 04/32] ASoC: qdsp6: q6afe: Increase APR timeout Wesley Cheng
2024-06-10 23:57 ` [PATCH v23 05/32] ASoC: qcom: qdsp6: Add USB backend ASoC driver for Q6 Wesley Cheng
2024-06-10 23:57 ` [PATCH v23 06/32] ALSA: usb-audio: Introduce USB SND platform op callbacks Wesley Cheng
2024-06-10 23:57 ` [PATCH v23 07/32] ALSA: usb-audio: Export USB SND APIs for modules Wesley Cheng
2024-06-10 23:57 ` [PATCH v23 08/32] ALSA: usb-audio: Save UAC sample size information Wesley Cheng
2024-06-10 23:57 ` [PATCH v23 09/32] usb: dwc3: Specify maximum number of XHCI interrupters Wesley Cheng
2024-06-10 23:57 ` [PATCH v23 10/32] usb: host: xhci-plat: Set XHCI max interrupters if property is present Wesley Cheng
2024-06-10 23:57 ` [PATCH v23 11/32] ALSA: usb-audio: qcom: Add USB QMI definitions Wesley Cheng
2024-06-10 23:57 ` [PATCH v23 12/32] ALSA: usb-audio: qcom: Introduce QC USB SND offloading support Wesley Cheng
2024-06-10 23:57 ` [PATCH v23 13/32] ALSA: usb-audio: Check for support for requested audio format Wesley Cheng
2024-06-10 23:57 ` [PATCH v23 14/32] ASoC: usb: Add PCM format check API for USB backend Wesley Cheng
2024-06-10 23:57 ` [PATCH v23 15/32] ASoC: qcom: qdsp6: Ensure PCM format is supported by USB audio device Wesley Cheng
2024-06-10 23:57 ` [PATCH v23 16/32] ALSA: usb-audio: Prevent starting of audio stream if in use Wesley Cheng
2024-06-10 23:57 ` [PATCH v23 17/32] ALSA: usb-audio: Do not allow USB offload path if PCM device is " Wesley Cheng
2024-06-12 14:57   ` Amadeusz Sławiński
2024-06-10 23:57 ` [PATCH v23 18/32] ASoC: dt-bindings: Update example for enabling USB offload on SM8250 Wesley Cheng
2024-06-10 23:57 ` [PATCH v23 19/32] ALSA: usb-audio: qcom: Populate PCM and USB chip information Wesley Cheng
2024-06-10 23:57 ` [PATCH v23 20/32] ASoC: qcom: qdsp6: Add support to track available USB PCM devices Wesley Cheng
2024-06-10 23:57 ` [PATCH v23 21/32] ASoC: Introduce SND kcontrols to select sound card and PCM device Wesley Cheng
2024-06-10 23:57 ` [PATCH v23 22/32] ASoC: qcom: qdsp6: Add SOC USB offload select get/put callbacks Wesley Cheng
2024-06-10 23:57 ` [PATCH v23 23/32] ASoC: Introduce SND kcontrols to track USB offloading state Wesley Cheng
2024-06-10 23:58 ` [PATCH v23 24/32] ASoC: qcom: qdsp6: Add PCM ops to track current state Wesley Cheng
2024-06-10 23:58 ` [PATCH v23 25/32] ASoC: usb: Create SOC USB SND jack kcontrol Wesley Cheng
2024-06-10 23:58 ` [PATCH v23 26/32] ASoC: qcom: qdsp6: Add headphone jack for offload connection status Wesley Cheng
2024-06-10 23:58 ` [PATCH v23 27/32] ASoC: usb: Fetch ASoC sound card information Wesley Cheng
2024-06-10 23:58 ` [PATCH v23 28/32] ALSA: usb-audio: Add USB offloading capable kcontrol Wesley Cheng
2024-06-10 23:58 ` [PATCH v23 29/32] ALSA: usb-audio: Allow for rediscovery of connected USB SND devices Wesley Cheng
2024-06-10 23:58 ` [PATCH v23 30/32] ALSA: usb-audio: qcom: Use card and PCM index from QMI request Wesley Cheng
2024-06-10 23:58 ` [PATCH v23 31/32] ASoC: usb: Rediscover USB SND devices on USB port add Wesley Cheng
2024-06-10 23:58 ` [PATCH v23 32/32] ASoC: doc: Add documentation for SOC USB Wesley Cheng
2024-06-12 12:25   ` Bagas Sanjaya
2024-06-12 19:30     ` Wesley Cheng
2024-06-12 14:47   ` Amadeusz Sławiński
2024-06-12 19:28     ` Wesley Cheng
2024-06-13  7:46       ` Amadeusz Sławiński
2024-06-17 17:02         ` Wesley Cheng
2024-06-18 11:42           ` Amadeusz Sławiński
2024-06-18 20:52             ` Wesley Cheng
2024-06-19  7:52               ` Amadeusz Sławiński [this message]
2024-06-20 22:04                 ` Wesley Cheng
2024-06-21  8:27                   ` Pierre-Louis Bossart
2024-06-28  0:59                     ` Wesley Cheng
2024-07-02  8:30                       ` Pierre-Louis Bossart
2024-07-02 23:34                         ` Wesley Cheng
2024-07-03  8:49                           ` Pierre-Louis Bossart
2024-07-03  9:13                             ` Amadeusz Sławiński
2024-07-03  9:50                               ` Pierre-Louis Bossart
2024-07-03 22:05                                 ` Wesley Cheng
2024-07-04 10:01                                   ` Amadeusz Sławiński
2024-07-04 11:25                                     ` Pierre-Louis Bossart
2024-07-08 23:16                                       ` Wesley Cheng
2024-07-26 19:52                                         ` Wesley Cheng
2024-06-12 14:50 ` [PATCH v23 00/32] Introduce QC USB SND audio offloading support Amadeusz Sławiński
2024-06-12 14:59   ` Pierre-Louis Bossart
2024-06-12 19:41     ` Wesley Cheng
2024-06-12 19:38   ` Wesley Cheng

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=510468c7-b181-48d0-bf2d-3e478b2f2aca@linux.intel.com \
    --to=amadeuszx.slawinski@linux.intel.com \
    --cc=Thinh.Nguyen@synopsys.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=bgoswami@quicinc.com \
    --cc=broonie@kernel.org \
    --cc=conor+dt@kernel.org \
    --cc=corbet@lwn.net \
    --cc=devicetree@vger.kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=krzk+dt@kernel.org \
    --cc=lgirdwood@gmail.com \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-sound@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=mathias.nyman@intel.com \
    --cc=perex@perex.cz \
    --cc=quic_wcheng@quicinc.com \
    --cc=robh@kernel.org \
    --cc=srinivas.kandagatla@linaro.org \
    --cc=tiwai@suse.com \
    /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;
as well as URLs for NNTP newsgroup(s).