Message ID | 20240425215125.29761-1-quic_wcheng@quicinc.com |
---|---|
Headers | show |
Series | Introduce QC USB SND audio offloading support | expand |
On Thu, Apr 25, 2024 at 02:51:25PM -0700, Wesley Cheng wrote: > With the introduction of the soc-usb driver, add documentation highlighting > details on how to utilize the new driver and how it interacts with > different components in USB SND and ASoC. It provides examples on how to > implement the drivers that will need to be introduced in order to enable > USB audio offloading. > The doc LGTM, thanks! Reviewed-by: Bagas Sanjaya <bagasdotme@gmail.com>
On 4/25/2024 11:50 PM, Wesley Cheng wrote: > Some clients may operate only on a specific XHCI interrupter instance. > Allow for the associated class driver to request for the interrupter that > it requires. > > Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com> > --- (...) > - > - /* Find available secondary interrupter, interrupter 0 is reserved for primary */ > + /* Find available secondary interrupter, interrupter 0 is reserverd for primary */ You introduce a typo in here.
On 4/25/2024 11:50 PM, Wesley Cheng wrote: > Some platforms may have support for offloading USB audio devices to a > dedicated audio DSP. Introduce a set of APIs that allow for management of > USB sound card and PCM devices enumerated by the USB SND class driver. > This allows for the ASoC components to be aware of what USB devices are > available for offloading. > > Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com> > --- (...) > +const char *snd_soc_usb_get_components_tag(bool playback) > +{ > + if (playback) > + return "usbplybkoffld: 1"; > + else > + return "usbcapoffld: 1"; > +} > +EXPORT_SYMBOL_GPL(snd_soc_usb_get_components_tag); Is this used to expose some information to userspace? Can those be some more readable strings if so, like: usbplaybackoffload, usbcaptureoffload (...) > + > + node = snd_soc_find_phandle(usbdev); > + if (IS_ERR(node)) > + return -ENODEV; > + > + ctx = snd_soc_find_usb_ctx(node); > + of_node_put(node); > + if (!ctx) > + return -ENODEV; Perhaps introduce some helper function, you do this snd_soc_find_phandle() followed by snd_soc_find_usb_ctx() in few places...
On 4/25/2024 11:51 PM, Wesley Cheng wrote: > Introduce a helper to check if a particular PCM format is supported by the > USB audio device connected. If the USB audio device does not have an > audio profile which can support the requested format, then notify the USB > backend. > > Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com> > --- (...) > +/** > + * snd_soc_usb_find_format() - Check if audio format is supported > + * @card_idx: USB sound chip array index > + * @params: PCM parameters > + * @direction: capture or playback > + * > + * Ensure that a requested audio profile from the ASoC side is able to be > + * supported by the USB device. > + * > + * Return 0 on success, negative on error. > + * > + */ > +int snd_soc_usb_find_format(int card_idx, struct snd_pcm_hw_params *params, > + int direction) Perhaps name function similar to its snd_usb equivalent, so snd_soc_usb_find_supported_format? > +{ > + struct snd_usb_stream *as; > + > + as = snd_usb_find_suppported_substream(card_idx, params, direction); > + if (!as) > + return -EOPNOTSUPP; > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(snd_soc_usb_find_format); > + > /** > * snd_soc_usb_allocate_port() - allocate a SOC USB device > * @component: USB DPCM backend DAI component >
Hi Amadeusz, On 4/26/2024 6:24 AM, Amadeusz Sławiński wrote: > On 4/25/2024 11:50 PM, Wesley Cheng wrote: >> Some clients may operate only on a specific XHCI interrupter instance. >> Allow for the associated class driver to request for the interrupter that >> it requires. >> >> Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com> >> --- > > (...) > >> - >> - /* Find available secondary interrupter, interrupter 0 is >> reserved for primary */ >> + /* Find available secondary interrupter, interrupter 0 is >> reserverd for primary */ > > You introduce a typo in here. Thanks for the review! Will fix it. Thanks Wesley Cheng
Hi Amadeusz, On 4/26/2024 6:25 AM, Amadeusz Sławiński wrote: > On 4/25/2024 11:50 PM, Wesley Cheng wrote: >> Some platforms may have support for offloading USB audio devices to a >> dedicated audio DSP. Introduce a set of APIs that allow for >> management of >> USB sound card and PCM devices enumerated by the USB SND class driver. >> This allows for the ASoC components to be aware of what USB devices are >> available for offloading. >> >> Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com> >> --- > > (...) > >> +const char *snd_soc_usb_get_components_tag(bool playback) >> +{ >> + if (playback) >> + return "usbplybkoffld: 1"; >> + else >> + return "usbcapoffld: 1"; >> +} >> +EXPORT_SYMBOL_GPL(snd_soc_usb_get_components_tag); > > Is this used to expose some information to userspace? > Can those be some more readable strings if so, like: > usbplaybackoffload, usbcaptureoffload > Sure we can make it a bit more complete. Was trying to keep it short, but if the intention isn't clear on the tag, then we can keep the full form. > (...) > >> + >> + node = snd_soc_find_phandle(usbdev); >> + if (IS_ERR(node)) >> + return -ENODEV; >> + >> + ctx = snd_soc_find_usb_ctx(node); >> + of_node_put(node); >> + if (!ctx) >> + return -ENODEV; > > Perhaps introduce some helper function, you do this > snd_soc_find_phandle() followed by snd_soc_find_usb_ctx() in few places... > Will do. Will make a helper and replace instances with this. Thanks Wesley Cheng
Hi Amadeusz, On 4/26/2024 6:25 AM, Amadeusz Sławiński wrote: > On 4/25/2024 11:51 PM, Wesley Cheng wrote: >> Introduce a helper to check if a particular PCM format is supported by >> the >> USB audio device connected. If the USB audio device does not have an >> audio profile which can support the requested format, then notify the USB >> backend. >> >> Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com> >> --- > > (...) > >> +/** >> + * snd_soc_usb_find_format() - Check if audio format is supported >> + * @card_idx: USB sound chip array index >> + * @params: PCM parameters >> + * @direction: capture or playback >> + * >> + * Ensure that a requested audio profile from the ASoC side is able >> to be >> + * supported by the USB device. >> + * >> + * Return 0 on success, negative on error. >> + * >> + */ >> +int snd_soc_usb_find_format(int card_idx, struct snd_pcm_hw_params >> *params, >> + int direction) > > Perhaps name function similar to its snd_usb equivalent, so > snd_soc_usb_find_supported_format? > Will do. Thanks Wesley Cheng