diff mbox series

[v21,26/39] ASoC: qcom: qdsp6: Add support to track available USB PCM devices

Message ID 20240507195116.9464-27-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
The USB backend should know about which sound resources are being shared
between the ASoC and USB SND paths.  This can be utilized to properly
select and maintain the offloading devices.

Signed-off-by: Wesley Cheng <quic_wcheng@quicinc.com>
---
 sound/soc/qcom/qdsp6/q6usb.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

Comments

Pierre-Louis Bossart May 7, 2024, 9:23 p.m. UTC | #1
> @@ -113,6 +120,12 @@ static int q6usb_alsa_connection_cb(struct snd_soc_usb *usb,
>  	if (connected) {
>  		/* We only track the latest USB headset plugged in */
>  		data->active_usb_chip_idx = sdev->card_idx;
> +
> +		set_bit(sdev->card_idx, &data->available_card_slot);
> +		data->status[sdev->card_idx].sdev = sdev;

Not following the 'only track the latest USB headset plugged in', I
don't see anything that discard the previously latest headset...

If you plug headset1, then headset2, how is headset1 marked as not
available for USB offload?

> +	} else {
> +		clear_bit(sdev->card_idx, &data->available_card_slot);
> +		data->status[sdev->card_idx].sdev = NULL;
>  	}
>  
>  	return 0;
Wesley Cheng May 8, 2024, 11:57 p.m. UTC | #2
Hi Pierre,

On 5/7/2024 2:23 PM, Pierre-Louis Bossart wrote:
> 
>> @@ -113,6 +120,12 @@ static int q6usb_alsa_connection_cb(struct snd_soc_usb *usb,
>>   	if (connected) {
>>   		/* We only track the latest USB headset plugged in */
>>   		data->active_usb_chip_idx = sdev->card_idx;
>> +
>> +		set_bit(sdev->card_idx, &data->available_card_slot);
>> +		data->status[sdev->card_idx].sdev = sdev;
> 
> Not following the 'only track the latest USB headset plugged in', I
> don't see anything that discard the previously latest headset...
> 
> If you plug headset1, then headset2, how is headset1 marked as not
> available for USB offload?
> 

It won't mark headset1 as not available for offload, because offload 
could happen on either depending on what is selected (from the kcontrol 
as well).

Thanks
Wesley Cheng
Pierre-Louis Bossart May 9, 2024, 1:02 p.m. UTC | #3
On 5/8/24 18:57, Wesley Cheng wrote:
> Hi Pierre,
> 
> On 5/7/2024 2:23 PM, Pierre-Louis Bossart wrote:
>>
>>> @@ -113,6 +120,12 @@ static int q6usb_alsa_connection_cb(struct
>>> snd_soc_usb *usb,
>>>       if (connected) {
>>>           /* We only track the latest USB headset plugged in */
>>>           data->active_usb_chip_idx = sdev->card_idx;
>>> +
>>> +        set_bit(sdev->card_idx, &data->available_card_slot);
>>> +        data->status[sdev->card_idx].sdev = sdev;
>>
>> Not following the 'only track the latest USB headset plugged in', I
>> don't see anything that discard the previously latest headset...
>>
>> If you plug headset1, then headset2, how is headset1 marked as not
>> available for USB offload?
>>
> 
> It won't mark headset1 as not available for offload, because offload
> could happen on either depending on what is selected (from the kcontrol
> as well).

Right, so the wording 'only track the latest USB headset plugged in' is
incorrect or obsolete, isn't it?
Wesley Cheng May 9, 2024, 9:49 p.m. UTC | #4
Hi Pierre,

On 5/9/2024 6:02 AM, Pierre-Louis Bossart wrote:
> 
> 
> On 5/8/24 18:57, Wesley Cheng wrote:
>> Hi Pierre,
>>
>> On 5/7/2024 2:23 PM, Pierre-Louis Bossart wrote:
>>>
>>>> @@ -113,6 +120,12 @@ static int q6usb_alsa_connection_cb(struct
>>>> snd_soc_usb *usb,
>>>>        if (connected) {
>>>>            /* We only track the latest USB headset plugged in */
>>>>            data->active_usb_chip_idx = sdev->card_idx;
>>>> +
>>>> +        set_bit(sdev->card_idx, &data->available_card_slot);
>>>> +        data->status[sdev->card_idx].sdev = sdev;
>>>
>>> Not following the 'only track the latest USB headset plugged in', I
>>> don't see anything that discard the previously latest headset...
>>>
>>> If you plug headset1, then headset2, how is headset1 marked as not
>>> available for USB offload?
>>>
>>
>> It won't mark headset1 as not available for offload, because offload
>> could happen on either depending on what is selected (from the kcontrol
>> as well).
> 
> Right, so the wording 'only track the latest USB headset plugged in' is
> incorrect or obsolete, isn't it?

Sure, I can reword it.  Will specify that it
"selects the latest USB headset plugged in for offloading"

Thanks
Wesley Cheng
diff mbox series

Patch

diff --git a/sound/soc/qcom/qdsp6/q6usb.c b/sound/soc/qcom/qdsp6/q6usb.c
index ee082d2fe786..1e85636bc6cb 100644
--- a/sound/soc/qcom/qdsp6/q6usb.c
+++ b/sound/soc/qcom/qdsp6/q6usb.c
@@ -27,11 +27,18 @@ 
 
 #define Q6_USB_SID_MASK	0xF
 
+struct q6usb_status {
+	struct snd_soc_usb_device *sdev;
+	unsigned int pcm_index;
+};
+
 struct q6usb_port_data {
 	struct q6afe_usb_cfg usb_cfg;
 	struct snd_soc_usb *usb;
 	struct q6usb_offload priv;
 	int active_usb_chip_idx;
+	unsigned long available_card_slot;
+	struct q6usb_status status[SNDRV_CARDS];
 };
 
 static const struct snd_soc_dapm_widget q6usb_dai_widgets[] = {
@@ -113,6 +120,12 @@  static int q6usb_alsa_connection_cb(struct snd_soc_usb *usb,
 	if (connected) {
 		/* We only track the latest USB headset plugged in */
 		data->active_usb_chip_idx = sdev->card_idx;
+
+		set_bit(sdev->card_idx, &data->available_card_slot);
+		data->status[sdev->card_idx].sdev = sdev;
+	} else {
+		clear_bit(sdev->card_idx, &data->available_card_slot);
+		data->status[sdev->card_idx].sdev = NULL;
 	}
 
 	return 0;