[v9,30/34] ASoC: qcom: qdsp6: Add SND kcontrol for fetching offload status

Message ID 20231017200109.11407-31-quic_wcheng@quicinc.com
State New
Headers
Series Introduce QC USB SND audio offloading support |

Commit Message

Wesley Cheng Oct. 17, 2023, 8:01 p.m. UTC
  Add a kcontrol to the platform sound card to fetch the current offload
status.  This can allow for userspace to ensure/check which USB SND
resources are actually busy versus having to attempt opening the USB SND
devices, which will result in an error if offloading is active.

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

Comments

Pierre-Louis Bossart Oct. 17, 2023, 10:53 p.m. UTC | #1
On 10/17/23 15:01, Wesley Cheng wrote:
> Add a kcontrol to the platform sound card to fetch the current offload
> status.  This can allow for userspace to ensure/check which USB SND
> resources are actually busy versus having to attempt opening the USB SND
> devices, which will result in an error if offloading is active.

I think I mentioned this a while back, but why not add the status in the
USB card itself? That's a generic component that all userspace agent
could query. Having a QCOM-specific control doesn't make the life of
userspace easier IMHO.
  
Wesley Cheng Oct. 19, 2023, 1:41 a.m. UTC | #2
Hi Pierre,

On 10/17/2023 3:53 PM, Pierre-Louis Bossart wrote:
> 
> 
> On 10/17/23 15:01, Wesley Cheng wrote:
>> Add a kcontrol to the platform sound card to fetch the current offload
>> status.  This can allow for userspace to ensure/check which USB SND
>> resources are actually busy versus having to attempt opening the USB SND
>> devices, which will result in an error if offloading is active.
> 
> I think I mentioned this a while back, but why not add the status in the
> USB card itself? That's a generic component that all userspace agent
> could query. Having a QCOM-specific control doesn't make the life of
> userspace easier IMHO.
> 
> 

Will take a look at this based on the comments you had in the other 
kcontrol patch.  Seeing if we can move it to a more generic layer.

Thanks
Wesley Cheng
  
Wesley Cheng Oct. 19, 2023, 7:25 p.m. UTC | #3
Hi Pierre,

On 10/18/2023 6:41 PM, Wesley Cheng wrote:
> Hi Pierre,
> 
> On 10/17/2023 3:53 PM, Pierre-Louis Bossart wrote:
>>
>>
>> On 10/17/23 15:01, Wesley Cheng wrote:
>>> Add a kcontrol to the platform sound card to fetch the current offload
>>> status.  This can allow for userspace to ensure/check which USB SND
>>> resources are actually busy versus having to attempt opening the USB SND
>>> devices, which will result in an error if offloading is active.
>>
>> I think I mentioned this a while back, but why not add the status in the
>> USB card itself? That's a generic component that all userspace agent
>> could query. Having a QCOM-specific control doesn't make the life of
>> userspace easier IMHO.
>>
>>
> 
> Will take a look at this based on the comments you had in the other 
> kcontrol patch.  Seeing if we can move it to a more generic layer.
> 

I think it would make more sense to see if we can keep all the offload 
kcontrols under the sound card exposed by the platform.  Especially, if 
we are going to modify the components string of the card to signify that 
it supports USB offload.

Thanks
Wesley Cheng
  
Pierre-Louis Bossart Oct. 19, 2023, 8:39 p.m. UTC | #4
>>>> Add a kcontrol to the platform sound card to fetch the current offload
>>>> status.  This can allow for userspace to ensure/check which USB SND
>>>> resources are actually busy versus having to attempt opening the USB
>>>> SND
>>>> devices, which will result in an error if offloading is active.
>>>
>>> I think I mentioned this a while back, but why not add the status in the
>>> USB card itself? That's a generic component that all userspace agent
>>> could query. Having a QCOM-specific control doesn't make the life of
>>> userspace easier IMHO.
>>>
>>>
>>
>> Will take a look at this based on the comments you had in the other
>> kcontrol patch.  Seeing if we can move it to a more generic layer.
>>
> 
> I think it would make more sense to see if we can keep all the offload
> kcontrols under the sound card exposed by the platform.  Especially, if
> we are going to modify the components string of the card to signify that
> it supports USB offload.

A two-way relationship would be ideal, i.e.
- the USB card has an indicator that it's currently bound with another
"platform" card that offers optimized offloaded capabilities.
- the platform card has a indicator that it exposes one or more PCM
devices routed to the USB card endpoint.

Android/HAL would typically start with the latter while other more
generic solutions would start from the former.
  

Patch

diff --git a/sound/soc/qcom/qdsp6/q6usb.c b/sound/soc/qcom/qdsp6/q6usb.c
index a95276b7d91d..d2f60ce66cf3 100644
--- a/sound/soc/qcom/qdsp6/q6usb.c
+++ b/sound/soc/qcom/qdsp6/q6usb.c
@@ -30,6 +30,8 @@  struct q6usb_status {
 	unsigned int num_pcm;
 	unsigned int chip_index;
 	unsigned int pcm_index;
+	bool prepared;
+	bool running;
 };
 
 struct q6usb_port_data {
@@ -52,6 +54,17 @@  static const struct snd_soc_dapm_route q6usb_dapm_routes[] = {
 	{"USB Playback", NULL, "USB_RX_BE"},
 };
 
+static int q6usb_find_running(struct q6usb_port_data *data)
+{
+	int i;
+
+	for (i = 0; i < SNDRV_CARDS; i++) {
+		if (data->status[i].running)
+			return i;
+	}
+	return -ENODEV;
+}
+
 static int q6usb_hw_params(struct snd_pcm_substream *substream,
 			   struct snd_pcm_hw_params *params,
 			   struct snd_soc_dai *dai)
@@ -81,14 +94,40 @@  static int q6usb_hw_params(struct snd_pcm_substream *substream,
 		goto out;
 
 	data->status[data->sel_card_idx].pcm_index = data->sel_pcm_idx;
+	data->status[data->sel_card_idx].prepared = true;
 out:
 	mutex_unlock(&data->mutex);
 
 	return ret;
 }
 
+static int q6usb_prepare(struct snd_pcm_substream *substream,
+		struct snd_soc_dai *dai)
+{
+	struct q6usb_port_data *data = dev_get_drvdata(dai->dev);
+
+	mutex_lock(&data->mutex);
+	data->status[data->sel_card_idx].running = true;
+	mutex_unlock(&data->mutex);
+
+	return 0;
+}
+
+static void q6usb_shutdown(struct snd_pcm_substream *substream,
+				struct snd_soc_dai *dai)
+{
+	struct q6usb_port_data *data = dev_get_drvdata(dai->dev);
+
+	mutex_lock(&data->mutex);
+	data->status[data->sel_card_idx].running = false;
+	data->status[data->sel_card_idx].prepared = false;
+	mutex_unlock(&data->mutex);
+}
+
 static const struct snd_soc_dai_ops q6usb_ops = {
 	.hw_params = q6usb_hw_params,
+	.prepare = q6usb_prepare,
+	.shutdown = q6usb_shutdown,
 };
 
 static struct snd_soc_dai_driver q6usb_be_dais[] = {
@@ -148,10 +187,15 @@  static int q6usb_put_offload_dev(struct snd_kcontrol *kcontrol,
 	int pcmidx;
 	int cardidx;
 
+	mutex_lock(&data->mutex);
+
+	/* Don't allow changes to the offloading devices if session is busy */
+	if (data->sel_card_idx >= 0 && data->status[data->sel_card_idx].prepared)
+		goto out;
+
 	cardidx = ucontrol->value.integer.value[0];
 	pcmidx = ucontrol->value.integer.value[1];
 
-	mutex_lock(&data->mutex);
 	if ((cardidx >= 0 && test_bit(cardidx, &data->available_card_slot))) {
 		data->sel_card_idx = cardidx;
 		changed = 1;
@@ -162,6 +206,8 @@  static int q6usb_put_offload_dev(struct snd_kcontrol *kcontrol,
 		data->idx_valid = true;
 		changed = 1;
 	}
+
+out:
 	mutex_unlock(&data->mutex);
 
 	return changed;
@@ -187,11 +233,59 @@  static const struct snd_kcontrol_new q6usb_offload_dev_ctrl = {
 	.put = q6usb_put_offload_dev,
 };
 
+static int q6usb_mixer_get_offload_status(struct snd_kcontrol *kcontrol,
+				   struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
+	struct q6usb_port_data *data = dev_get_drvdata(component->dev);
+	int running;
+	int card_idx;
+	int pcm_idx;
+
+	running = q6usb_find_running(data);
+	if (running < 0) {
+		card_idx = -1;
+		pcm_idx = -1;
+	} else {
+		card_idx = running;
+		pcm_idx = data->status[running].pcm_index;
+	}
+
+	ucontrol->value.integer.value[0] = card_idx;
+	ucontrol->value.integer.value[1] = pcm_idx;
+	return 0;
+}
+
+static int q6usb_offload_ctl_info(struct snd_kcontrol *kcontrol,
+			      struct snd_ctl_elem_info *uinfo)
+{
+	uinfo->type = SNDRV_CTL_ELEM_TYPE_INTEGER;
+	uinfo->count = 2;
+	uinfo->value.integer.min = 0;
+	uinfo->value.integer.max = SNDRV_CARDS;
+
+	return 0;
+}
+
+static const struct snd_kcontrol_new q6usb_offload_control = {
+	.iface = SNDRV_CTL_ELEM_IFACE_CARD,
+	.access = SNDRV_CTL_ELEM_ACCESS_READ,
+	.name = "Q6USB offload status",
+	.info = q6usb_offload_ctl_info,
+	.get = q6usb_mixer_get_offload_status,
+	.put = NULL,
+};
+
 /* Build a mixer control for a UAC connector control (jack-detect) */
 static void q6usb_connector_control_init(struct snd_soc_component *component)
 {
 	int ret;
 
+	ret = snd_ctl_add(component->card->snd_card,
+				snd_ctl_new1(&q6usb_offload_control, component));
+	if (ret < 0)
+		return;
+
 	ret = snd_ctl_add(component->card->snd_card,
 				snd_ctl_new1(&q6usb_offload_dev_ctrl, component));
 	if (ret < 0)
@@ -229,8 +323,12 @@  static int q6usb_alsa_connection_cb(struct snd_soc_usb *usb,
 
 	mutex_lock(&data->mutex);
 	if (connected) {
-		/* We only track the latest USB headset plugged in */
-		if (!data->idx_valid || data->sel_card_idx < 0)
+		/*
+		 * Update the latest USB headset plugged in, if session is
+		 * idle.
+		 */
+		if ((!data->idx_valid || data->sel_card_idx < 0) &&
+			!data->status[data->sel_card_idx].prepared)
 			data->sel_card_idx = sdev->card_idx;
 
 		set_bit(sdev->card_idx, &data->available_card_slot);