lkml.org 
[lkml]   [2023]   [Feb]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [RFC PATCH v2 12/22] sound: usb: card: Introduce USB SND platform op callbacks
    From
    Hi Greg,

    On 2/27/2023 11:30 PM, Greg KH wrote:
    > On Mon, Feb 27, 2023 at 06:59:32PM -0800, Wesley Cheng wrote:
    >> Hi Greg,
    >>
    >> On 2/10/2023 2:49 PM, Wesley Cheng wrote:
    >>> Hi Greg,
    >>>
    >>> On 1/28/2023 5:28 AM, Greg KH wrote:
    >>>> On Wed, Jan 25, 2023 at 07:14:14PM -0800, Wesley Cheng wrote:
    >>>>> Allow for different platforms to be notified on USB SND
    >>>>> connect/disconnect
    >>>>> seqeunces.  This allows for platform USB SND modules to properly
    >>>>> initialize
    >>>>> and populate internal structures with references to the USB SND chip
    >>>>> device.
    >>>>>
    >>>>> Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com>
    >>>>> ---
    >>>>>   sound/usb/card.c | 28 ++++++++++++++++++++++++++++
    >>>>>   sound/usb/card.h | 20 ++++++++++++++++++++
    >>>>>   2 files changed, 48 insertions(+)
    >>>>>
    >>>>> diff --git a/sound/usb/card.c b/sound/usb/card.c
    >>>>> index 26268ffb8274..803230343c16 100644
    >>>>> --- a/sound/usb/card.c
    >>>>> +++ b/sound/usb/card.c
    >>>>> @@ -117,6 +117,24 @@ MODULE_PARM_DESC(skip_validation, "Skip
    >>>>> unit descriptor validation (default: no)
    >>>>>   static DEFINE_MUTEX(register_mutex);
    >>>>>   static struct snd_usb_audio *usb_chip[SNDRV_CARDS];
    >>>>>   static struct usb_driver usb_audio_driver;
    >>>>> +static struct snd_usb_platform_ops *platform_ops;
    >>>>
    >>>> You can not have a single "platform_ops" pointer, this HAS to be
    >>>> per-bus.
    >>>>
    >>>
    >>> Agreed.
    >>>
    >>
    >> I looked at seeing how we could implement this at a per bus level, but the
    >> USB class driver model doesn't exactly have a good framework for supporting
    >> this. Reason being is because, at the time of the USB SND class driver
    >> initialization, there is a big chance that there isn't a USB bus registered
    >> in the system, so the point of adding the operations is not clear. However,
    >> we need to ensure that we've added the platform/driver operations before any
    >> USB SND devices are detected.
    >
    > But the offload "engine" is associated with the specific USB bus
    > controller instance in the system, so perhaps you are just not adding
    > this to the correct location?
    >

    There are several parts to the offload logic:
    1. XHCI interrupter/resource components - fetching addresses to the
    proper event ring and transfer rings for the audio DSP. This is the
    part which is specific to the controller instance, and APIs are being
    directly exported from the XHCI HCD, as the offloading features utilized
    are only specific for XHCI based controllers. This is handled in
    patches 1-6 in this series. Each XHCI instance will have its own set of
    interrupters, and transfer resources.

    2. USB offload class driver - driver which interacts with USB SND for
    operations like UAC descriptor parsing, USB audio device support params,
    and USB endpoint setup (ie issuing SET_INTERFACE to enable the device to
    start playback this is a SETUP transaction). It will interact with the
    USB backend and items in #1, to set up the audio playback.

    > The sound core shouldn't care about this at all, add the logic to the
    > USB host controller driver instead, why isn't this just another USB bus
    > function?
    >

    The intention of the platform ops here is to mainly keep track of USB
    SND card/pcm device creation, and access to the main "struct
    snd_usb_audio". This structure carries all the information about the
    different substreams allocated, as well as the formats supported by the
    audio device. This is passed onto the USB backend, which will be
    utilized in my next revision to allow userspace to specifically select
    the proper card/PCM device to enable offload on.

    >> To add to the above, in case of OTG/DRD (dual role) designs, the USB HCD/bus
    >> isn't created until we move into the host role. At that time, using DWC3 as
    >> an example, we will create the XHCI platform device, and probe the USB HCD,
    >> where a USB bus is created.
    >
    > Great, again, tie it to the specific xhci host controler instance.
    >
    >> In general, we currently think this USB offload driver should co-exist with
    >> the USB SND class driver, which handles all devices connected across every
    >> bus.
    >
    > And that is incorrect, please do not do that.
    >

    To clarify, I think we can summarize that the qc_audio_offload driver
    (the one that registers the platform operations) is mainly responsible
    for USB SND card management, and communicating that to the USB backend.

    >> We can add a check to the platform connect routine to ensure that
    >> there is a reference to the USB backend. If so, then that particular USB
    >> bus/sysdev can be supported by the audio DSP. That way, we do not falsely
    >> populate USB SND cards which are present on another USB bus/controller.
    >
    > You should NEVER be able to populate a USB card unless the USB bus
    > controller has given you the USB interface structure to control, so I do
    > not understand how this is an issue.
    >

    This might not be so clear with the current revision. In the next
    revision I have prepared, as I mentioned, we are proposing using the
    platform connect/disconnect ops here to build a reference to all USB SND
    card and PCM devices available in the system.

    For example, if you have an external hub connected to the root hub, and
    each port on that hub has an audio device connected. We want to allow
    userspace to select which card# and pcm# to start offload on. (versus
    userspace having to rely on trail and error, which Pierre touched on on
    why that is not desired)

    Since the USB SND driver is based on udevs (not bus specific), if there
    are multiple roothubs (correlates to usb buses), then we only want to
    notify the USB backend of the USB SND cards on the controller which has
    offloading enabled. Otherwise, if userspace selects a device on an
    unsupported controller, then that path would obviously fail.

    I hope that clarifies some things.

    Thanks
    Wesley Cheng

    \
     
     \ /
      Last update: 2023-03-27 00:38    [W:4.424 / U:0.852 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site