diff mbox series

[v21,38/39] ASoC: qcom: Populate SoC components string

Message ID 20240507195116.9464-39-quic_wcheng@quicinc.com (mailing list archive)
State Superseded
Headers show
Series Introduce QC USB SND audio offloading support | expand

Commit Message

Wesley Cheng May 7, 2024, 7:51 p.m. UTC
For userspace to know about certain capabilities of the current platform
card, add tags to the components string that it can use to enable support
for that audio path.  In case of USB offloading, the "usboffldplybk: 1" tag
is added to the string.

Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com>
---
 sound/soc/qcom/sm8250.c            |  3 +++
 sound/soc/qcom/usb_offload_utils.c | 22 ++++++++++++++++++++++
 sound/soc/qcom/usb_offload_utils.h |  7 +++++++
 3 files changed, 32 insertions(+)

Comments

Pierre-Louis Bossart May 7, 2024, 9:40 p.m. UTC | #1
On 5/7/24 14:51, Wesley Cheng wrote:
> For userspace to know about certain capabilities of the current platform
> card, add tags to the components string that it can use to enable support
> for that audio path.  In case of USB offloading, the "usboffldplybk: 1" tag

usboffloadplayback?

same question as before, do we need spaces?

And if we have controls, why do we need component strings? The component
string is not dynamic to the best of my knowledge, this could be
problematic if the card is no longer capable of supporting this stream,
while a control can be updated at will.
Wesley Cheng May 8, 2024, 8:06 p.m. UTC | #2
Hi Pierre,

On 5/7/2024 2:40 PM, Pierre-Louis Bossart wrote:
> 
> 
> On 5/7/24 14:51, Wesley Cheng wrote:
>> For userspace to know about certain capabilities of the current platform
>> card, add tags to the components string that it can use to enable support
>> for that audio path.  In case of USB offloading, the "usboffldplybk: 1" tag
> 
> usboffloadplayback?
> 
> same question as before, do we need spaces?
> 

I think spaces are currently used as a delimiter, so I'll remove the spaces.

> And if we have controls, why do we need component strings? The component
> string is not dynamic to the best of my knowledge, this could be
> problematic if the card is no longer capable of supporting this stream,
> while a control can be updated at will.
> 

Maybe I misunderstood your comment here:

https://lore.kernel.org/linux-usb/925d7c03-c288-49a4-8bcd-395b32810d75@linux.intel.com/

At the time, I didn't include the kcontrols on the USB SND portion of 
it, which was added after this series.  My interpretation was that there 
were userspace entities that could query for general information about 
what the card supports based on the components string, or sound card 
name.  I treated this as an independent identifier, since the change to 
add the offload capable jack was present.

Thanks
Wesley Cheng
Pierre-Louis Bossart May 9, 2024, 1:17 p.m. UTC | #3
On 5/8/24 15:06, Wesley Cheng wrote:
> Hi Pierre,
> 
> On 5/7/2024 2:40 PM, Pierre-Louis Bossart wrote:
>>
>>
>> On 5/7/24 14:51, Wesley Cheng wrote:
>>> For userspace to know about certain capabilities of the current platform
>>> card, add tags to the components string that it can use to enable
>>> support
>>> for that audio path.  In case of USB offloading, the "usboffldplybk:
>>> 1" tag
>>
>> usboffloadplayback?
>>
>> same question as before, do we need spaces?
>>
> 
> I think spaces are currently used as a delimiter, so I'll remove the
> spaces.
> 
>> And if we have controls, why do we need component strings? The component
>> string is not dynamic to the best of my knowledge, this could be
>> problematic if the card is no longer capable of supporting this stream,
>> while a control can be updated at will.
>>
> 
> Maybe I misunderstood your comment here:
> 
> https://lore.kernel.org/linux-usb/925d7c03-c288-49a4-8bcd-395b32810d75@linux.intel.com/
> 
> At the time, I didn't include the kcontrols on the USB SND portion of
> it, which was added after this series.  My interpretation was that there
> were userspace entities that could query for general information about
> what the card supports based on the components string, or sound card
> name.  I treated this as an independent identifier, since the change to
> add the offload capable jack was present.

My comment at the time stands: it's very hard to figure out that a
random card supports USB and is connected to a given endpoint.

It'd be much easier as you wrote in the comments on patch 34 to have a
control in the "regular" USB card to point to the 'better' offloaded
path exposed by another card. Applications wouldn't need to know what
this other card is, they would then use the card:device information
directly.
Wesley Cheng May 9, 2024, 10:09 p.m. UTC | #4
Hi Pierre,

On 5/9/2024 6:17 AM, Pierre-Louis Bossart wrote:
> 
> 
> On 5/8/24 15:06, Wesley Cheng wrote:
>> Hi Pierre,
>>
>> On 5/7/2024 2:40 PM, Pierre-Louis Bossart wrote:
>>>
>>>
>>> On 5/7/24 14:51, Wesley Cheng wrote:
>>>> For userspace to know about certain capabilities of the current platform
>>>> card, add tags to the components string that it can use to enable
>>>> support
>>>> for that audio path.  In case of USB offloading, the "usboffldplybk:
>>>> 1" tag
>>>
>>> usboffloadplayback?
>>>
>>> same question as before, do we need spaces?
>>>
>>
>> I think spaces are currently used as a delimiter, so I'll remove the
>> spaces.
>>
>>> And if we have controls, why do we need component strings? The component
>>> string is not dynamic to the best of my knowledge, this could be
>>> problematic if the card is no longer capable of supporting this stream,
>>> while a control can be updated at will.
>>>
>>
>> Maybe I misunderstood your comment here:
>>
>> https://lore.kernel.org/linux-usb/925d7c03-c288-49a4-8bcd-395b32810d75@linux.intel.com/
>>
>> At the time, I didn't include the kcontrols on the USB SND portion of
>> it, which was added after this series.  My interpretation was that there
>> were userspace entities that could query for general information about
>> what the card supports based on the components string, or sound card
>> name.  I treated this as an independent identifier, since the change to
>> add the offload capable jack was present.
> 
> My comment at the time stands: it's very hard to figure out that a
> random card supports USB and is connected to a given endpoint.
> 
> It'd be much easier as you wrote in the comments on patch 34 to have a
> control in the "regular" USB card to point to the 'better' offloaded
> path exposed by another card. Applications wouldn't need to know what
> this other card is, they would then use the card:device information
> directly.

OK, then it might be fine to remove the components tag if patch#34 is 
there.  That kcontrol is exposed as part of the sound card created for 
the USB device, so if applications queried, it would signify that there 
is an offload path available.

For this kcontrol, it will return the ASoC platform card index, would 
that be sufficient?

Thanks
Wesley Cheng
diff mbox series

Patch

diff --git a/sound/soc/qcom/sm8250.c b/sound/soc/qcom/sm8250.c
index 6f156c94de01..dc17f7fd3a2b 100644
--- a/sound/soc/qcom/sm8250.c
+++ b/sound/soc/qcom/sm8250.c
@@ -39,6 +39,9 @@  static int sm8250_snd_init(struct snd_soc_pcm_runtime *rtd)
 						&data->usb_offload_jack_setup);
 	else
 		ret = qcom_snd_wcd_jack_setup(rtd, &data->jack, &data->jack_setup);
+
+	qcom_snd_add_components_string(rtd);
+
 	return ret;
 }
 
diff --git a/sound/soc/qcom/usb_offload_utils.c b/sound/soc/qcom/usb_offload_utils.c
index a84394a9b772..2d2ce0687b01 100644
--- a/sound/soc/qcom/usb_offload_utils.c
+++ b/sound/soc/qcom/usb_offload_utils.c
@@ -52,4 +52,26 @@  int qcom_snd_usb_offload_jack_remove(struct snd_soc_pcm_runtime *rtd,
 	return 0;
 }
 EXPORT_SYMBOL_GPL(qcom_snd_usb_offload_jack_remove);
+
+int qcom_snd_add_components_string(struct snd_soc_pcm_runtime *rtd)
+{
+	struct snd_soc_dai *cpu_dai = snd_soc_rtd_to_cpu(rtd, 0);
+	struct snd_soc_card *card = rtd->card;
+	const char *prev = card->components;
+
+	switch (cpu_dai->id) {
+	case USB_RX:
+		card->components = devm_kasprintf(card->dev, GFP_KERNEL, "%s %s",
+					  card->components,
+					  snd_soc_usb_get_components_tag(true));
+	default:
+		break;
+	}
+
+	if (prev && prev != card->components)
+		devm_kfree(card->dev, prev);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(qcom_snd_add_components_string);
 MODULE_LICENSE("GPL");
diff --git a/sound/soc/qcom/usb_offload_utils.h b/sound/soc/qcom/usb_offload_utils.h
index d44f78569b47..814fbfa26d36 100644
--- a/sound/soc/qcom/usb_offload_utils.h
+++ b/sound/soc/qcom/usb_offload_utils.h
@@ -13,6 +13,8 @@  int qcom_snd_usb_offload_jack_setup(struct snd_soc_pcm_runtime *rtd,
 
 int qcom_snd_usb_offload_jack_remove(struct snd_soc_pcm_runtime *rtd,
 					bool *jack_setup);
+
+int qcom_snd_add_components_string(struct snd_soc_pcm_runtime *rtd);
 #else
 static inline int qcom_snd_usb_offload_jack_setup(struct snd_soc_pcm_runtime *rtd,
 			    struct snd_soc_jack *jack, bool *jack_setup)
@@ -25,5 +27,10 @@  static inline int qcom_snd_usb_offload_jack_remove(struct snd_soc_pcm_runtime *r
 {
 	return -ENODEV;
 }
+
+static inline int qcom_snd_add_components_string(struct snd_soc_pcm_runtime *rtd)
+{
+	return -ENODEV;
+}
 #endif /* IS_ENABLED(CONFIG_SND_SOC_QCOM_OFFLOAD_UTILS) */
 #endif /* __QCOM_SND_USB_OFFLOAD_UTILS_H__ */