Re: RFC: usb: gadget: u_audio: Notifying gadget that host started playback/capture?

[Pavel Hofman <pavel.hofman@ivitera.com>]

[-- Attachment #1: Type: text/plain, Size: 2097 bytes --]

Hi,

Dne 08. 09. 21 v 10:21 Pavel Hofman napsal(a):
> Hi,
> 
> The current audio gadget has no way to inform the gadget side that the 
> host side has started playback/capture and that gadget-side alsa 
> processes should be started.
> 
> Playback/capture processes on the host side do not get stuck without the 
> gadget side consuming/producing data (OUT requests are ignored in 
> u_audio_iso_complete, IN ones send initial zeros in their req->buf).
> 
> However, playback/capture processes on the gadget side get stuck without 
> the host side sending playback OUT packets or capture IN requests and 
> time out with error. If there was a way to inform the gadget side that 
> playback/capture has started on the host side, the gadget clients could 
> react accordingly.
> 

I drafted a simple patch for u_audio.c which defines read-only boolean 
ctl elems "Capture Requested" and "Playback Requested". Their values are 
set/reset in methods u_audio_start_capture/playback and 
u_audio_stop_capture/playback, i.e. at changes of respective altsettings 
from 0 to 1 and back. Every ctl elem value change sends notification via 
snd_ctl_notify. The principle works OK for capture/playback start/stop 
on the host, as monitored by alsactl:

pi@raspberrypi:~ $ alsactl monitor hw:UAC2Gadget
node hw:UAC2Gadget, #4 (3,0,0,Capture Requested,0) VALUE
node hw:UAC2Gadget, #4 (3,0,0,Capture Requested,0) VALUE
node hw:UAC2Gadget, #3 (3,0,0,Playback Requested,0) VALUE
node hw:UAC2Gadget, #3 (3,0,0,Playback Requested,0) VALUE

However at enumeration the USB host switches both playback and capture 
altsettings repeatedly, generating "fake" events from the gadget side 
POW. The host even sends regular-sized EP-OUT packets filled with zeros 
during enumeration (tested on linux only for now).

Please is there any way to "detect" the enumeration stage to mask out 
the "fake" playback/capture start/stop events?

The attached patch does not apply cleanly to mainline u_audio.c because 
it's rebased on other patches not submitted yet but it's only a 
discussion inducer for now.

Thanks a lot,

Pavel.

[-- Attachment #2: patch.diff --]
[-- Type: text/x-patch, Size: 5116 bytes --]

diff --git a/drivers/usb/gadget/function/u_audio.c b/drivers/usb/gadget/function/u_audio.c
index e395be32275c..69aef4e3677d 100644
--- a/drivers/usb/gadget/function/u_audio.c
+++ b/drivers/usb/gadget/function/u_audio.c
@@ -34,6 +34,8 @@ enum {
 	UAC_VOLUME_CTRL,
 	UAC_CAPTURE_RATE_CTRL,
 	UAC_PLAYBACK_RATE_CTRL,
+	UAC_CAPTURE_REQ_CTRL,
+	UAC_PLAYBACK_REQ_CTRL,
 };
 
 /* Runtime data params for one stream */
@@ -63,6 +65,9 @@ struct uac_rtd_params {
   s16 volume_min, volume_max, volume_res;
   s16 volume;
   int mute;
+  /* playback/capture_is_requested and their state */
+  struct snd_kcontrol *snd_kctl_is_req;
+  int is_requested;
 
   spinlock_t lock; /* lock for control transfers */
 
@@ -542,6 +547,8 @@ int u_audio_set_playback_srate(struct g_audio *audio_dev, int srate)
 }
 EXPORT_SYMBOL_GPL(u_audio_set_playback_srate);
 
+static void update_is_requested(struct uac_rtd_params *prm, unsigned int val);
+
 int u_audio_start_capture(struct g_audio *audio_dev)
 {
 	struct snd_uac_chip *uac = audio_dev->uac;
@@ -553,6 +560,7 @@ int u_audio_start_capture(struct g_audio *audio_dev)
 	struct uac_params *params = &audio_dev->params;
 	int req_len, i;
 
+	update_is_requested(&uac->c_prm, 1);
 	ep = audio_dev->out_ep;
 	prm = &uac->c_prm;
 	config_ep_by_speed(gadget, &audio_dev->func, ep);
@@ -625,6 +633,7 @@ void u_audio_stop_capture(struct g_audio *audio_dev)
 {
 	struct snd_uac_chip *uac = audio_dev->uac;
 
+	update_is_requested(&uac->c_prm, 0);
 	if (audio_dev->in_ep_fback)
 		free_ep_fback(&uac->c_prm, audio_dev->in_ep_fback);
 	free_ep(&uac->c_prm, audio_dev->out_ep);
@@ -645,6 +654,7 @@ int u_audio_start_playback(struct g_audio *audio_dev)
 	int req_len, i;
 	unsigned int p_interval, p_pktsize, p_pktsize_residue;
 
+	update_is_requested(&uac->p_prm, 1);
 	dev_dbg(dev, "start playback with rate %d\n", params->p_srate_active);
 	ep = audio_dev->in_ep;
 	prm = &uac->p_prm;
@@ -711,6 +721,7 @@ void u_audio_stop_playback(struct g_audio *audio_dev)
 {
 	struct snd_uac_chip *uac = audio_dev->uac;
 
+	update_is_requested(&uac->p_prm, 0);
 	free_ep(&uac->p_prm, audio_dev->in_ep);
 }
 EXPORT_SYMBOL_GPL(u_audio_stop_playback);
@@ -1027,6 +1038,27 @@ static int uac_pcm_rate_get(struct snd_kcontrol *kcontrol,
 	return 0;
 }
 
+static int u_audio_stream_requested_info(struct snd_kcontrol *kcontrol,
+				   struct snd_ctl_elem_info *uinfo)
+{
+	uinfo->type = SNDRV_CTL_ELEM_TYPE_BOOLEAN;
+	uinfo->count = 1;
+	uinfo->value.integer.min = 0;
+	uinfo->value.integer.max = 1;
+	uinfo->value.integer.step = 1;
+
+	return 0;
+}
+
+static int u_audio_stream_requested_get(struct snd_kcontrol *kcontrol,
+				   struct snd_ctl_elem_value *ucontrol)
+{
+	struct uac_rtd_params *prm = snd_kcontrol_chip(kcontrol);
+	ucontrol->value.integer.value[0] = prm->is_requested;
+	return 0;
+}
+
+
 static struct snd_kcontrol_new u_audio_controls[]  = {
   [UAC_FBACK_CTRL] {
     .iface =        SNDRV_CTL_ELEM_IFACE_PCM,
@@ -1072,8 +1104,35 @@ static struct snd_kcontrol_new u_audio_controls[]  = {
 		.get = uac_pcm_rate_get,
 		.private_value = SNDRV_PCM_STREAM_PLAYBACK,
 		},
+  [UAC_CAPTURE_REQ_CTRL] {
+    .iface =        SNDRV_CTL_ELEM_IFACE_PCM,
+    .name =         "Capture Requested",
+    .access = SNDRV_CTL_ELEM_ACCESS_READ | SNDRV_CTL_ELEM_ACCESS_VOLATILE,
+    .info =         u_audio_stream_requested_info,
+    .get =          u_audio_stream_requested_get,
+    .private_value = SNDRV_PCM_STREAM_CAPTURE,
+  },
+  [UAC_PLAYBACK_REQ_CTRL] {
+    .iface =        SNDRV_CTL_ELEM_IFACE_PCM,
+    .name =         "Playback Requested",
+    .access = SNDRV_CTL_ELEM_ACCESS_READ | SNDRV_CTL_ELEM_ACCESS_VOLATILE,
+    .info =         u_audio_stream_requested_info,
+    .get =          u_audio_stream_requested_get,
+    .private_value = SNDRV_PCM_STREAM_PLAYBACK,
+  },
 };
 
+static void update_is_requested(struct uac_rtd_params *prm, unsigned int val) {
+	struct snd_kcontrol *kctl = prm->snd_kctl_is_req;
+
+	if (prm->is_requested != val) {
+		prm->is_requested = val;
+		snd_ctl_notify(prm->uac->card, SNDRV_CTL_EVENT_MASK_VALUE,
+				&kctl->id);
+		pr_debug("Setting '%s' to %d", kctl->id.name, val);
+	}
+}
+
 int g_audio_setup(struct g_audio *g_audio, const char *pcm_name,
 					const char *card_name)
 {
@@ -1209,6 +1268,38 @@ int g_audio_setup(struct g_audio *g_audio, const char *pcm_name,
 		err = snd_ctl_add(card, kctl);
 		if (err < 0)
 			goto snd_fail;
+
+		kctl = snd_ctl_new1(&u_audio_controls[UAC_PLAYBACK_REQ_CTRL],
+						&uac->p_prm);
+		if (!kctl) {
+			err = -ENOMEM;
+			goto snd_fail;
+		}
+
+		kctl->id.device = pcm->device;
+		kctl->id.subdevice = 0;
+
+		err = snd_ctl_add(card, kctl);
+		if (err < 0)
+			goto snd_fail;
+		(&uac->p_prm)->snd_kctl_is_req = kctl;
+	}
+
+	if (c_chmask) {
+		kctl = snd_ctl_new1(&u_audio_controls[UAC_CAPTURE_REQ_CTRL],
+					&uac->c_prm);
+		if (!kctl) {
+			err = -ENOMEM;
+			goto snd_fail;
+		}
+
+		kctl->id.device = pcm->device;
+		kctl->id.subdevice = 0;
+
+		err = snd_ctl_add(card, kctl);
+		if (err < 0)
+			goto snd_fail;
+		(&uac->c_prm)->snd_kctl_is_req = kctl;
 	}
 
 	for (i = 0; i <= SNDRV_PCM_STREAM_LAST; i++) {