* [PATCH 0/3] UAC2 Gadget: feedback endpoint support
@ 2020-11-08 0:18 Ruslan Bilovol
2020-11-08 0:18 ` [PATCH 1/3] usb: gadget: f_uac2/u_audio: add " Ruslan Bilovol
` (3 more replies)
0 siblings, 4 replies; 31+ messages in thread
From: Ruslan Bilovol @ 2020-11-08 0:18 UTC (permalink / raw)
To: balbi; +Cc: linux-usb, gschmottlach
Current UAC2 gadget implements capture/sync paths
as two USB ISO ASYNC endpoints (IN and OUT).
This violates USB spec which says that ISO ASYNC OUT endpoint
should have feedback companion endpoint.
See USB2.0 spec "5.12.4.1 Synchronization Type": asynchronous
sink provides explicit feedback (isochronous pipe).
Interesting that for ISO ASYNC *IN* endpoint respective
feedback isn't required since source provides implicit
feedforward (data stream).
While it's not an issue if UAC2 Gadget is connected to
Linux host (Linux ignores missing feedback endpoint),
with other hosts like Windows or MacOS the UAC2 Gadget
isn't enumerated due to missing feedback endpoint.
This patch series adds feedback endpoint support to
UAC2 function, new control to UAC2 mixer which can
be used by userspace tools (like alsaloop from alsa-utils)
for updating feedback frequency reported to the host.
This is useful for usecases when UAC2 Gadget's audio
samples are played to another codec or audio card
with its own internal freerunning clock so host can
be notified that more/less samples are required.
The alsaloop tool requires some (relatively small)
modifications in order to start support driving
feedback frequency through UAC2 mixer control.
That change will be sent as a separate patch
to ALSA community.
Also added ability to switch ISO ASYNC OUT endpoint into
adaptive endpoint which doesn't require feedback endpoint
(as per USB spec).
Ruslan Bilovol (3):
usb: gadget: f_uac2/u_audio: add feedback endpoint support
usb: gadget: f_uac2: add adaptive sync support for capture
usb: gadget: u_audio: add real feedback implementation
Documentation/ABI/testing/configfs-usb-gadget-uac2 | 1 +
Documentation/usb/gadget-testing.rst | 1 +
drivers/usb/gadget/function/f_uac2.c | 118 ++++++++++-
drivers/usb/gadget/function/u_audio.c | 217 ++++++++++++++++++++-
drivers/usb/gadget/function/u_audio.h | 10 +
drivers/usb/gadget/function/u_uac2.h | 2 +
6 files changed, 345 insertions(+), 4 deletions(-)
--
1.9.1
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 1/3] usb: gadget: f_uac2/u_audio: add feedback endpoint support
2020-11-08 0:18 [PATCH 0/3] UAC2 Gadget: feedback endpoint support Ruslan Bilovol
@ 2020-11-08 0:18 ` Ruslan Bilovol
2020-11-11 9:26 ` Peter Chen
2020-11-08 0:18 ` [PATCH 2/3] usb: gadget: f_uac2: add adaptive sync support for capture Ruslan Bilovol
` (2 subsequent siblings)
3 siblings, 1 reply; 31+ messages in thread
From: Ruslan Bilovol @ 2020-11-08 0:18 UTC (permalink / raw)
To: balbi; +Cc: linux-usb, gschmottlach
As per USB and UAC2 specs, asynchronous audio sink endpoint
requires explicit synchronization mechanism (Isochronous
Feedback Endpoint)
Implement feedback companion endpoint for ISO OUT endpoint
This patch adds all required infrastructure and USB requests
handling for feedback endpoint. Syncrhonization itself is
still dummy (feedback ep always reports 'nomimal frequency'
e.g. no adjustement is needed). This satisfies hosts that
require feedback endpoint (like Win10) and poll it periodically
Actual synchronization mechanism should be implemented
separately
Signed-off-by: Ruslan Bilovol <ruslan.bilovol@gmail.com>
---
drivers/usb/gadget/function/f_uac2.c | 34 +++++++++-
drivers/usb/gadget/function/u_audio.c | 124 +++++++++++++++++++++++++++++++++-
drivers/usb/gadget/function/u_audio.h | 3 +
3 files changed, 158 insertions(+), 3 deletions(-)
diff --git a/drivers/usb/gadget/function/f_uac2.c b/drivers/usb/gadget/function/f_uac2.c
index fb9b875..a57bf77 100644
--- a/drivers/usb/gadget/function/f_uac2.c
+++ b/drivers/usb/gadget/function/f_uac2.c
@@ -237,7 +237,7 @@ enum {
.bDescriptorType = USB_DT_INTERFACE,
.bAlternateSetting = 1,
- .bNumEndpoints = 1,
+ .bNumEndpoints = 2,
.bInterfaceClass = USB_CLASS_AUDIO,
.bInterfaceSubClass = USB_SUBCLASS_AUDIOSTREAMING,
.bInterfaceProtocol = UAC_VERSION_2,
@@ -296,6 +296,27 @@ enum {
.wLockDelay = 0,
};
+/* STD AS ISO IN Feedback Endpoint */
+static struct usb_endpoint_descriptor fs_epin_fback_desc = {
+ .bLength = USB_DT_ENDPOINT_SIZE,
+ .bDescriptorType = USB_DT_ENDPOINT,
+
+ .bEndpointAddress = USB_DIR_IN,
+ .bmAttributes = USB_ENDPOINT_XFER_ISOC | USB_ENDPOINT_USAGE_FEEDBACK,
+ .wMaxPacketSize = cpu_to_le16(3),
+ .bInterval = 1,
+};
+
+static struct usb_endpoint_descriptor hs_epin_fback_desc = {
+ .bLength = USB_DT_ENDPOINT_SIZE,
+ .bDescriptorType = USB_DT_ENDPOINT,
+
+ .bmAttributes = USB_ENDPOINT_XFER_ISOC | USB_ENDPOINT_USAGE_FEEDBACK,
+ .wMaxPacketSize = cpu_to_le16(4),
+ .bInterval = 4,
+};
+
+
/* Audio Streaming IN Interface - Alt0 */
static struct usb_interface_descriptor std_as_in_if0_desc = {
.bLength = sizeof std_as_in_if0_desc,
@@ -392,6 +413,7 @@ enum {
(struct usb_descriptor_header *)&as_out_fmt1_desc,
(struct usb_descriptor_header *)&fs_epout_desc,
(struct usb_descriptor_header *)&as_iso_out_desc,
+ (struct usb_descriptor_header *)&fs_epin_fback_desc,
(struct usb_descriptor_header *)&std_as_in_if0_desc,
(struct usb_descriptor_header *)&std_as_in_if1_desc,
@@ -422,6 +444,7 @@ enum {
(struct usb_descriptor_header *)&as_out_fmt1_desc,
(struct usb_descriptor_header *)&hs_epout_desc,
(struct usb_descriptor_header *)&as_iso_out_desc,
+ (struct usb_descriptor_header *)&hs_epin_fback_desc,
(struct usb_descriptor_header *)&std_as_in_if0_desc,
(struct usb_descriptor_header *)&std_as_in_if1_desc,
@@ -541,6 +564,7 @@ static void setup_descriptor(struct f_uac2_opts *opts)
fs_audio_desc[i++] = USBDHDR(&as_out_fmt1_desc);
fs_audio_desc[i++] = USBDHDR(&fs_epout_desc);
fs_audio_desc[i++] = USBDHDR(&as_iso_out_desc);
+ fs_audio_desc[i++] = USBDHDR(&fs_epin_fback_desc);
}
if (EPIN_EN(opts)) {
fs_audio_desc[i++] = USBDHDR(&std_as_in_if0_desc);
@@ -574,6 +598,7 @@ static void setup_descriptor(struct f_uac2_opts *opts)
hs_audio_desc[i++] = USBDHDR(&as_out_fmt1_desc);
hs_audio_desc[i++] = USBDHDR(&hs_epout_desc);
hs_audio_desc[i++] = USBDHDR(&as_iso_out_desc);
+ hs_audio_desc[i++] = USBDHDR(&hs_epin_fback_desc);
}
if (EPIN_EN(opts)) {
hs_audio_desc[i++] = USBDHDR(&std_as_in_if0_desc);
@@ -681,6 +706,12 @@ static void setup_descriptor(struct f_uac2_opts *opts)
dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
return -ENODEV;
}
+ agdev->in_ep_fback = usb_ep_autoconfig(gadget,
+ &fs_epin_fback_desc);
+ if (!agdev->in_ep_fback) {
+ dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
+ return -ENODEV;
+ }
}
if (EPIN_EN(uac2_opts)) {
@@ -699,6 +730,7 @@ static void setup_descriptor(struct f_uac2_opts *opts)
le16_to_cpu(hs_epout_desc.wMaxPacketSize));
hs_epout_desc.bEndpointAddress = fs_epout_desc.bEndpointAddress;
+ hs_epin_fback_desc.bEndpointAddress = fs_epin_fback_desc.bEndpointAddress;
hs_epin_desc.bEndpointAddress = fs_epin_desc.bEndpointAddress;
setup_descriptor(uac2_opts);
diff --git a/drivers/usb/gadget/function/u_audio.c b/drivers/usb/gadget/function/u_audio.c
index e6d32c5..33c9288 100644
--- a/drivers/usb/gadget/function/u_audio.c
+++ b/drivers/usb/gadget/function/u_audio.c
@@ -43,6 +43,10 @@ struct uac_rtd_params {
unsigned int max_psize; /* MaxPacketSize of endpoint */
struct uac_req *ureq;
+ struct usb_request *req_fback; /* Feedback endpoint request */
+ unsigned int ffback; /* Real frequency reported by feedback endpoint */
+ bool fb_ep_enabled; /* if the ep is enabled */
+
spinlock_t lock;
};
@@ -76,6 +80,35 @@ struct snd_uac_chip {
.periods_min = MIN_PERIODS,
};
+static void u_audio_set_fback_frequency(enum usb_device_speed speed,
+ unsigned int freq, void *buf)
+{
+ u32 ff = 0;
+
+ if (speed == USB_SPEED_FULL) {
+ /*
+ * Full-speed feedback endpoints report frequency
+ * in samples/microframe
+ * Format is encoded in Q10.10 left-justified in the 24 bits,
+ * so that it has a Q10.14 format.
+ */
+ ff = DIV_ROUND_UP((freq << 14), 1000);
+ } else {
+ /*
+ * High-speed feedback endpoints report frequency
+ * in samples/microframe.
+ * Format is encoded in Q12.13 fitted into four bytes so that
+ * the binary point is located between the second and the third
+ * byte fromat (that is Q16.16)
+ *
+ * Prevent integer overflow by calculating in Q12.13 fromat and
+ * then shifting to Q16.16
+ */
+ ff = DIV_ROUND_UP((freq << 13), (8*1000)) << 3;
+ }
+ *(__le32 *)buf = cpu_to_le32(ff);
+}
+
static void u_audio_iso_complete(struct usb_ep *ep, struct usb_request *req)
{
unsigned int pending;
@@ -182,6 +215,36 @@ static void u_audio_iso_complete(struct usb_ep *ep, struct usb_request *req)
dev_err(uac->card->dev, "%d Error!\n", __LINE__);
}
+static void u_audio_iso_fback_complete(struct usb_ep *ep,
+ struct usb_request *req)
+{
+ struct uac_rtd_params *prm = req->context;
+ struct snd_uac_chip *uac = prm->uac;
+ struct g_audio *audio_dev = uac->audio_dev;
+ int status = req->status;
+ unsigned long flags;
+
+ /* i/f shutting down */
+ if (!prm->fb_ep_enabled || req->status == -ESHUTDOWN)
+ return;
+
+ /*
+ * We can't really do much about bad xfers.
+ * Afterall, the ISOCH xfers could fail legitimately.
+ */
+ if (status)
+ pr_debug("%s: iso_complete status(%d) %d/%d\n",
+ __func__, status, req->actual, req->length);
+
+ spin_lock_irqsave(&prm->lock, flags);
+ u_audio_set_fback_frequency(audio_dev->gadget->speed,
+ prm->ffback, req->buf);
+ spin_unlock_irqrestore(&prm->lock, flags);
+
+ if (usb_ep_queue(ep, req, GFP_ATOMIC))
+ dev_err(uac->card->dev, "%d Error!\n", __LINE__);
+}
+
static int uac_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
{
struct snd_uac_chip *uac = snd_pcm_substream_chip(substream);
@@ -346,14 +409,33 @@ static inline void free_ep(struct uac_rtd_params *prm, struct usb_ep *ep)
dev_err(uac->card->dev, "%s:%d Error!\n", __func__, __LINE__);
}
+static inline void free_ep_fback(struct uac_rtd_params *prm, struct usb_ep *ep)
+{
+ struct snd_uac_chip *uac = prm->uac;
+
+ if (!prm->fb_ep_enabled)
+ return;
+
+ prm->fb_ep_enabled = false;
+
+ if (prm->req_fback) {
+ usb_ep_dequeue(ep, prm->req_fback);
+ kfree(prm->req_fback->buf);
+ usb_ep_free_request(ep, prm->req_fback);
+ prm->req_fback = NULL;
+ }
+
+ if (usb_ep_disable(ep))
+ dev_err(uac->card->dev, "%s:%d Error!\n", __func__, __LINE__);
+}
int u_audio_start_capture(struct g_audio *audio_dev)
{
struct snd_uac_chip *uac = audio_dev->uac;
struct usb_gadget *gadget = audio_dev->gadget;
struct device *dev = &gadget->dev;
- struct usb_request *req;
- struct usb_ep *ep;
+ struct usb_request *req, *req_fback;
+ struct usb_ep *ep, *ep_fback;
struct uac_rtd_params *prm;
struct uac_params *params = &audio_dev->params;
int req_len, i;
@@ -386,6 +468,42 @@ int u_audio_start_capture(struct g_audio *audio_dev)
dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
}
+ ep_fback = audio_dev->in_ep_fback;
+ if (!ep_fback)
+ return 0;
+
+ /* Setup feedback endpoint */
+ config_ep_by_speed(gadget, &audio_dev->func, ep_fback);
+ prm->fb_ep_enabled = true;
+ usb_ep_enable(ep_fback);
+ req_len = ep_fback->maxpacket;
+
+ req_fback = usb_ep_alloc_request(ep_fback, GFP_ATOMIC);
+ if (req_fback == NULL)
+ return -ENOMEM;
+
+ prm->req_fback = req_fback;
+ req_fback->zero = 0;
+ req_fback->context = prm;
+ req_fback->length = req_len;
+ req_fback->complete = u_audio_iso_fback_complete;
+
+ req_fback->buf = kzalloc(req_len, GFP_ATOMIC);
+ if (!req_fback->buf)
+ return -ENOMEM;
+
+ /*
+ * Configure the feedback endpoint's reported frequency.
+ * Always start with original frequency since its deviation can't
+ * be meauserd at start of playback
+ */
+ prm->ffback = params->c_srate;
+ u_audio_set_fback_frequency(audio_dev->gadget->speed,
+ prm->ffback, req_fback->buf);
+
+ if (usb_ep_queue(ep_fback, req_fback, GFP_ATOMIC))
+ dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
+
return 0;
}
EXPORT_SYMBOL_GPL(u_audio_start_capture);
@@ -394,6 +512,8 @@ void u_audio_stop_capture(struct g_audio *audio_dev)
{
struct snd_uac_chip *uac = audio_dev->uac;
+ 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);
}
EXPORT_SYMBOL_GPL(u_audio_stop_capture);
diff --git a/drivers/usb/gadget/function/u_audio.h b/drivers/usb/gadget/function/u_audio.h
index 5ea6b86..53e6baf 100644
--- a/drivers/usb/gadget/function/u_audio.h
+++ b/drivers/usb/gadget/function/u_audio.h
@@ -30,7 +30,10 @@ struct g_audio {
struct usb_gadget *gadget;
struct usb_ep *in_ep;
+
struct usb_ep *out_ep;
+ /* feedback IN endpoint corresponding to out_ep */
+ struct usb_ep *in_ep_fback;
/* Max packet size for all in_ep possible speeds */
unsigned int in_ep_maxpsize;
--
1.9.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH 2/3] usb: gadget: f_uac2: add adaptive sync support for capture
2020-11-08 0:18 [PATCH 0/3] UAC2 Gadget: feedback endpoint support Ruslan Bilovol
2020-11-08 0:18 ` [PATCH 1/3] usb: gadget: f_uac2/u_audio: add " Ruslan Bilovol
@ 2020-11-08 0:18 ` Ruslan Bilovol
2020-11-11 9:18 ` Peter Chen
2020-11-26 11:13 ` Jerome Brunet
2020-11-08 0:18 ` [PATCH 3/3] usb: gadget: u_audio: add real feedback implementation Ruslan Bilovol
2020-11-11 9:30 ` [PATCH 0/3] UAC2 Gadget: feedback endpoint support Peter Chen
3 siblings, 2 replies; 31+ messages in thread
From: Ruslan Bilovol @ 2020-11-08 0:18 UTC (permalink / raw)
To: balbi; +Cc: linux-usb, gschmottlach
Current f_uac2 USB OUT (aka 'capture') synchronization
implements 'ASYNC' scenario which means USB Gadget has
it's own freerunning clock and can update Host about
real clock frequency through feedback endpoint so Host
can align number of samples sent to the USB gadget to
prevent overruns/underruns
In case if Gadget can has no it's internal clock and
can consume audio samples at any rate (for example,
on the Gadget side someone records audio directly to
a file, or audio samples are played through an
external DAC as soon as they arrive), UAC2 spec
suggests 'ADAPTIVE' synchronization type.
Change UAC2 driver to make it configurable through
additional 'c_sync' configfs file.
Default remains 'asynchronous' with possibility to
switch it to 'adaptive'
Signed-off-by: Ruslan Bilovol <ruslan.bilovol@gmail.com>
---
Documentation/ABI/testing/configfs-usb-gadget-uac2 | 1 +
Documentation/usb/gadget-testing.rst | 1 +
drivers/usb/gadget/function/f_uac2.c | 96 ++++++++++++++++++++--
drivers/usb/gadget/function/u_uac2.h | 2 +
4 files changed, 91 insertions(+), 9 deletions(-)
diff --git a/Documentation/ABI/testing/configfs-usb-gadget-uac2 b/Documentation/ABI/testing/configfs-usb-gadget-uac2
index 2bfdd4e..4fbff96 100644
--- a/Documentation/ABI/testing/configfs-usb-gadget-uac2
+++ b/Documentation/ABI/testing/configfs-usb-gadget-uac2
@@ -7,6 +7,7 @@ Description:
c_chmask - capture channel mask
c_srate - capture sampling rate
c_ssize - capture sample size (bytes)
+ c_sync - capture synchronization type (async/adaptive)
p_chmask - playback channel mask
p_srate - playback sampling rate
p_ssize - playback sample size (bytes)
diff --git a/Documentation/usb/gadget-testing.rst b/Documentation/usb/gadget-testing.rst
index 2eeb3e9..360a7ca 100644
--- a/Documentation/usb/gadget-testing.rst
+++ b/Documentation/usb/gadget-testing.rst
@@ -728,6 +728,7 @@ The uac2 function provides these attributes in its function directory:
c_chmask capture channel mask
c_srate capture sampling rate
c_ssize capture sample size (bytes)
+ c_sync capture synchronization type (async/adaptive)
p_chmask playback channel mask
p_srate playback sampling rate
p_ssize playback sample size (bytes)
diff --git a/drivers/usb/gadget/function/f_uac2.c b/drivers/usb/gadget/function/f_uac2.c
index a57bf77..3187ad3 100644
--- a/drivers/usb/gadget/function/f_uac2.c
+++ b/drivers/usb/gadget/function/f_uac2.c
@@ -41,6 +41,7 @@
#define EPIN_EN(_opts) ((_opts)->p_chmask != 0)
#define EPOUT_EN(_opts) ((_opts)->c_chmask != 0)
+#define EPOUT_FBACK_IN_EN(_opts) ((_opts)->c_sync == USB_ENDPOINT_SYNC_ASYNC)
struct f_uac2 {
struct g_audio g_audio;
@@ -237,7 +238,7 @@ enum {
.bDescriptorType = USB_DT_INTERFACE,
.bAlternateSetting = 1,
- .bNumEndpoints = 2,
+ .bNumEndpoints = 1,
.bInterfaceClass = USB_CLASS_AUDIO,
.bInterfaceSubClass = USB_SUBCLASS_AUDIOSTREAMING,
.bInterfaceProtocol = UAC_VERSION_2,
@@ -270,7 +271,7 @@ enum {
.bDescriptorType = USB_DT_ENDPOINT,
.bEndpointAddress = USB_DIR_OUT,
- .bmAttributes = USB_ENDPOINT_XFER_ISOC | USB_ENDPOINT_SYNC_ASYNC,
+ .bmAttributes = USB_ENDPOINT_XFER_ISOC,
.wMaxPacketSize = cpu_to_le16(1023),
.bInterval = 1,
};
@@ -279,7 +280,7 @@ enum {
.bLength = USB_DT_ENDPOINT_SIZE,
.bDescriptorType = USB_DT_ENDPOINT,
- .bmAttributes = USB_ENDPOINT_XFER_ISOC | USB_ENDPOINT_SYNC_ASYNC,
+ .bmAttributes = USB_ENDPOINT_XFER_ISOC,
.wMaxPacketSize = cpu_to_le16(1024),
.bInterval = 1,
};
@@ -540,6 +541,19 @@ static void setup_descriptor(struct f_uac2_opts *opts)
len += sizeof(io_out_ot_desc);
ac_hdr_desc.wTotalLength = cpu_to_le16(len);
iad_desc.bInterfaceCount++;
+
+ if (EPOUT_FBACK_IN_EN(opts)) {
+ fs_epout_desc.bmAttributes |=
+ USB_ENDPOINT_SYNC_ASYNC;
+ hs_epout_desc.bmAttributes |=
+ USB_ENDPOINT_SYNC_ASYNC;
+ std_as_out_if1_desc.bNumEndpoints++;
+ } else {
+ fs_epout_desc.bmAttributes |=
+ USB_ENDPOINT_SYNC_ADAPTIVE;
+ hs_epout_desc.bmAttributes |=
+ USB_ENDPOINT_SYNC_ADAPTIVE;
+ }
}
i = 0;
@@ -564,7 +578,8 @@ static void setup_descriptor(struct f_uac2_opts *opts)
fs_audio_desc[i++] = USBDHDR(&as_out_fmt1_desc);
fs_audio_desc[i++] = USBDHDR(&fs_epout_desc);
fs_audio_desc[i++] = USBDHDR(&as_iso_out_desc);
- fs_audio_desc[i++] = USBDHDR(&fs_epin_fback_desc);
+ if (EPOUT_FBACK_IN_EN(opts))
+ fs_audio_desc[i++] = USBDHDR(&fs_epin_fback_desc);
}
if (EPIN_EN(opts)) {
fs_audio_desc[i++] = USBDHDR(&std_as_in_if0_desc);
@@ -598,7 +613,8 @@ static void setup_descriptor(struct f_uac2_opts *opts)
hs_audio_desc[i++] = USBDHDR(&as_out_fmt1_desc);
hs_audio_desc[i++] = USBDHDR(&hs_epout_desc);
hs_audio_desc[i++] = USBDHDR(&as_iso_out_desc);
- hs_audio_desc[i++] = USBDHDR(&hs_epin_fback_desc);
+ if (EPOUT_FBACK_IN_EN(opts))
+ hs_audio_desc[i++] = USBDHDR(&hs_epin_fback_desc);
}
if (EPIN_EN(opts)) {
hs_audio_desc[i++] = USBDHDR(&std_as_in_if0_desc);
@@ -706,11 +722,14 @@ static void setup_descriptor(struct f_uac2_opts *opts)
dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
return -ENODEV;
}
- agdev->in_ep_fback = usb_ep_autoconfig(gadget,
+ if (EPOUT_FBACK_IN_EN(uac2_opts)) {
+ agdev->in_ep_fback = usb_ep_autoconfig(gadget,
&fs_epin_fback_desc);
- if (!agdev->in_ep_fback) {
- dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
- return -ENODEV;
+ if (!agdev->in_ep_fback) {
+ dev_err(dev, "%s:%d Error!\n",
+ __func__, __LINE__);
+ return -ENODEV;
+ }
}
}
@@ -1057,11 +1076,68 @@ static void f_uac2_attr_release(struct config_item *item)
\
CONFIGFS_ATTR(f_uac2_opts_, name)
+#define UAC2_ATTRIBUTE_SYNC(name) \
+static ssize_t f_uac2_opts_##name##_show(struct config_item *item, \
+ char *page) \
+{ \
+ struct f_uac2_opts *opts = to_f_uac2_opts(item); \
+ int result; \
+ char *str; \
+ \
+ mutex_lock(&opts->lock); \
+ switch (opts->name) { \
+ case USB_ENDPOINT_SYNC_ASYNC: \
+ str = "async"; \
+ break; \
+ case USB_ENDPOINT_SYNC_ADAPTIVE: \
+ str = "adaptive"; \
+ break; \
+ default: \
+ str = "unknown"; \
+ break; \
+ } \
+ result = sprintf(page, "%s\n", str); \
+ mutex_unlock(&opts->lock); \
+ \
+ return result; \
+} \
+ \
+static ssize_t f_uac2_opts_##name##_store(struct config_item *item, \
+ const char *page, size_t len) \
+{ \
+ struct f_uac2_opts *opts = to_f_uac2_opts(item); \
+ int ret = 0; \
+ \
+ mutex_lock(&opts->lock); \
+ if (opts->refcnt) { \
+ ret = -EBUSY; \
+ goto end; \
+ } \
+ \
+ if (!strncmp(page, "async", 5)) \
+ opts->name = USB_ENDPOINT_SYNC_ASYNC; \
+ else if (!strncmp(page, "adaptive", 8)) \
+ opts->name = USB_ENDPOINT_SYNC_ADAPTIVE; \
+ else { \
+ ret = -EINVAL; \
+ goto end; \
+ } \
+ \
+ ret = len; \
+ \
+end: \
+ mutex_unlock(&opts->lock); \
+ return ret; \
+} \
+ \
+CONFIGFS_ATTR(f_uac2_opts_, name)
+
UAC2_ATTRIBUTE(p_chmask);
UAC2_ATTRIBUTE(p_srate);
UAC2_ATTRIBUTE(p_ssize);
UAC2_ATTRIBUTE(c_chmask);
UAC2_ATTRIBUTE(c_srate);
+UAC2_ATTRIBUTE_SYNC(c_sync);
UAC2_ATTRIBUTE(c_ssize);
UAC2_ATTRIBUTE(req_number);
@@ -1072,6 +1148,7 @@ static void f_uac2_attr_release(struct config_item *item)
&f_uac2_opts_attr_c_chmask,
&f_uac2_opts_attr_c_srate,
&f_uac2_opts_attr_c_ssize,
+ &f_uac2_opts_attr_c_sync,
&f_uac2_opts_attr_req_number,
NULL,
};
@@ -1110,6 +1187,7 @@ static struct usb_function_instance *afunc_alloc_inst(void)
opts->c_chmask = UAC2_DEF_CCHMASK;
opts->c_srate = UAC2_DEF_CSRATE;
opts->c_ssize = UAC2_DEF_CSSIZE;
+ opts->c_sync = UAC2_DEF_CSYNC;
opts->req_number = UAC2_DEF_REQ_NUM;
return &opts->func_inst;
}
diff --git a/drivers/usb/gadget/function/u_uac2.h b/drivers/usb/gadget/function/u_uac2.h
index b503571..13589c3 100644
--- a/drivers/usb/gadget/function/u_uac2.h
+++ b/drivers/usb/gadget/function/u_uac2.h
@@ -21,6 +21,7 @@
#define UAC2_DEF_CCHMASK 0x3
#define UAC2_DEF_CSRATE 64000
#define UAC2_DEF_CSSIZE 2
+#define UAC2_DEF_CSYNC USB_ENDPOINT_SYNC_ASYNC
#define UAC2_DEF_REQ_NUM 2
struct f_uac2_opts {
@@ -31,6 +32,7 @@ struct f_uac2_opts {
int c_chmask;
int c_srate;
int c_ssize;
+ int c_sync;
int req_number;
bool bound;
--
1.9.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH 3/3] usb: gadget: u_audio: add real feedback implementation
2020-11-08 0:18 [PATCH 0/3] UAC2 Gadget: feedback endpoint support Ruslan Bilovol
2020-11-08 0:18 ` [PATCH 1/3] usb: gadget: f_uac2/u_audio: add " Ruslan Bilovol
2020-11-08 0:18 ` [PATCH 2/3] usb: gadget: f_uac2: add adaptive sync support for capture Ruslan Bilovol
@ 2020-11-08 0:18 ` Ruslan Bilovol
2020-11-09 8:24 ` Pavel Hofman
2020-11-11 9:30 ` [PATCH 0/3] UAC2 Gadget: feedback endpoint support Peter Chen
3 siblings, 1 reply; 31+ messages in thread
From: Ruslan Bilovol @ 2020-11-08 0:18 UTC (permalink / raw)
To: balbi; +Cc: linux-usb, gschmottlach
This adds interface between userspace and feedback
endpoint to report real feedback frequency to the Host.
Current implementation adds new userspace interface
ALSA mixer control "PCM Feedback Frequency Hz" (similar
to aloop driver's "PCM Rate Shift 100000" mixer control)
We allow +/-20% deviation of nominal sampling frequency,
that usually is more than enough in real-world usecases
Usage of this new control is easy to implement in
existing userspace tools like alsaloop from alsa-utils.
Signed-off-by: Ruslan Bilovol <ruslan.bilovol@gmail.com>
---
drivers/usb/gadget/function/f_uac2.c | 4 ++
drivers/usb/gadget/function/u_audio.c | 93 +++++++++++++++++++++++++++++++++++
drivers/usb/gadget/function/u_audio.h | 7 +++
3 files changed, 104 insertions(+)
diff --git a/drivers/usb/gadget/function/f_uac2.c b/drivers/usb/gadget/function/f_uac2.c
index 3187ad3..ebdb2a1 100644
--- a/drivers/usb/gadget/function/f_uac2.c
+++ b/drivers/usb/gadget/function/f_uac2.c
@@ -487,6 +487,10 @@ static void set_ep_max_packet_size(const struct f_uac2_opts *uac2_opts,
max_packet_size = num_channels(chmask) * ssize *
DIV_ROUND_UP(srate, factor / (1 << (ep_desc->bInterval - 1)));
+
+ if (!is_playback && (uac2_opts->c_sync == USB_ENDPOINT_SYNC_ASYNC))
+ max_packet_size = max_packet_size * FBACK_FREQ_MAX / 100;
+
ep_desc->wMaxPacketSize = cpu_to_le16(min_t(u16, max_packet_size,
le16_to_cpu(ep_desc->wMaxPacketSize)));
}
diff --git a/drivers/usb/gadget/function/u_audio.c b/drivers/usb/gadget/function/u_audio.c
index 33c9288..ae9e1f3 100644
--- a/drivers/usb/gadget/function/u_audio.c
+++ b/drivers/usb/gadget/function/u_audio.c
@@ -16,6 +16,7 @@
#include <sound/core.h>
#include <sound/pcm.h>
#include <sound/pcm_params.h>
+#include <sound/control.h>
#include "u_audio.h"
@@ -596,12 +597,87 @@ void u_audio_stop_playback(struct g_audio *audio_dev)
}
EXPORT_SYMBOL_GPL(u_audio_stop_playback);
+static int u_audio_rate_shift_info(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_info *uinfo)
+{
+ struct uac_rtd_params *prm = snd_kcontrol_chip(kcontrol);
+ struct snd_uac_chip *uac = prm->uac;
+ struct g_audio *audio_dev = uac->audio_dev;
+ struct uac_params *params = &audio_dev->params;
+ unsigned int ffback_min, ffback_max;
+
+ ffback_min = params->c_srate * FBACK_FREQ_MIN / 100;
+ ffback_max = params->c_srate * FBACK_FREQ_MIN / 100;
+
+ uinfo->type = SNDRV_CTL_ELEM_TYPE_INTEGER;
+ uinfo->count = 1;
+ uinfo->value.integer.min = ffback_min;
+ uinfo->value.integer.max = ffback_max;
+ uinfo->value.integer.step = 1;
+ return 0;
+}
+
+static int u_audio_rate_shift_get(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ struct uac_rtd_params *prm = snd_kcontrol_chip(kcontrol);
+ unsigned long flags;
+
+ spin_lock_irqsave(&prm->lock, flags);
+ ucontrol->value.integer.value[0] = prm->ffback;
+ spin_unlock_irqrestore(&prm->lock, flags);
+
+ return 0;
+}
+
+static int u_audio_rate_shift_put(struct snd_kcontrol *kcontrol,
+ struct snd_ctl_elem_value *ucontrol)
+{
+ struct uac_rtd_params *prm = snd_kcontrol_chip(kcontrol);
+ struct snd_uac_chip *uac = prm->uac;
+ struct g_audio *audio_dev = uac->audio_dev;
+ struct uac_params *params = &audio_dev->params;
+ unsigned int val;
+ unsigned int ffback_min, ffback_max;
+ unsigned long flags;
+ int change = 0;
+
+ ffback_min = params->c_srate * FBACK_FREQ_MIN / 100;
+ ffback_max = params->c_srate * FBACK_FREQ_MAX / 100;
+
+ val = ucontrol->value.integer.value[0];
+ if (val < ffback_min)
+ val = ffback_min;
+ if (val > ffback_max)
+ val = ffback_max;
+
+ spin_lock_irqsave(&prm->lock, flags);
+ if (prm->ffback != val) {
+ prm->ffback = val;
+ change = 1;
+ }
+ spin_unlock_irqrestore(&prm->lock, flags);
+
+ return change;
+}
+
+static const struct snd_kcontrol_new u_audio_controls[] = {
+{
+ .iface = SNDRV_CTL_ELEM_IFACE_PCM,
+ .name = "PCM Feedback Frequency Hz",
+ .info = u_audio_rate_shift_info,
+ .get = u_audio_rate_shift_get,
+ .put = u_audio_rate_shift_put,
+},
+};
+
int g_audio_setup(struct g_audio *g_audio, const char *pcm_name,
const char *card_name)
{
struct snd_uac_chip *uac;
struct snd_card *card;
struct snd_pcm *pcm;
+ struct snd_kcontrol *kctl;
struct uac_params *params;
int p_chmask, c_chmask;
int err;
@@ -687,6 +763,23 @@ int g_audio_setup(struct g_audio *g_audio, const char *pcm_name,
snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_PLAYBACK, &uac_pcm_ops);
snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_CAPTURE, &uac_pcm_ops);
+ if (c_chmask && g_audio->in_ep_fback) {
+ strlcpy(card->mixername, card_name, sizeof(card->driver));
+
+ kctl = snd_ctl_new1(&u_audio_controls[0], &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;
+ }
+
strlcpy(card->driver, card_name, sizeof(card->driver));
strlcpy(card->shortname, card_name, sizeof(card->shortname));
sprintf(card->longname, "%s %i", card_name, card->dev->id);
diff --git a/drivers/usb/gadget/function/u_audio.h b/drivers/usb/gadget/function/u_audio.h
index 53e6baf..fd70808 100644
--- a/drivers/usb/gadget/function/u_audio.h
+++ b/drivers/usb/gadget/function/u_audio.h
@@ -11,6 +11,13 @@
#include <linux/usb/composite.h>
+/*
+ * Min/max percentage of nominal sampling frequency deviation
+ * reported through feedback endpoint to the host
+ */
+#define FBACK_FREQ_MIN 80
+#define FBACK_FREQ_MAX 120
+
struct uac_params {
/* playback */
int p_chmask; /* channel mask */
--
1.9.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH 3/3] usb: gadget: u_audio: add real feedback implementation
2020-11-08 0:18 ` [PATCH 3/3] usb: gadget: u_audio: add real feedback implementation Ruslan Bilovol
@ 2020-11-09 8:24 ` Pavel Hofman
2020-11-09 8:25 ` Pavel Hofman
2020-11-12 11:26 ` Pavel Hofman
0 siblings, 2 replies; 31+ messages in thread
From: Pavel Hofman @ 2020-11-09 8:24 UTC (permalink / raw)
To: Ruslan Bilovol, balbi; +Cc: linux-usb, gschmottlach
Hi Bilovol,
Dne 08. 11. 20 v 1:18 Ruslan Bilovol napsal(a):
> This adds interface between userspace and feedback
> endpoint to report real feedback frequency to the Host.
>
> Current implementation adds new userspace interface
> ALSA mixer control "PCM Feedback Frequency Hz" (similar
> to aloop driver's "PCM Rate Shift 100000" mixer control)
>
> We allow +/-20% deviation of nominal sampling frequency,
> that usually is more than enough in real-world usecases
>
> Usage of this new control is easy to implement in
> existing userspace tools like alsaloop from alsa-utils.
>
> Signed-off-by: Ruslan Bilovol <ruslan.bilovol@gmail.com>
> ---
> drivers/usb/gadget/function/f_uac2.c | 4 ++
> drivers/usb/gadget/function/u_audio.c | 93 +++++++++++++++++++++++++++++++++++
> drivers/usb/gadget/function/u_audio.h | 7 +++
> 3 files changed, 104 insertions(+)
Thanks a lot for the great implementation. IIUC the control element sets
integer frequency in Hz. Often the clocks deviate by small fractions of
Hz. Please have you considered the value to be e.g. in 100th of Hz for
finer control of the samplerate? Similar to the PCM Rate Shift which has
a step 100000th of the samplerate.
Thanks a lot.
Best regards,
Pavel.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 3/3] usb: gadget: u_audio: add real feedback implementation
2020-11-09 8:24 ` Pavel Hofman
@ 2020-11-09 8:25 ` Pavel Hofman
2020-11-12 11:26 ` Pavel Hofman
1 sibling, 0 replies; 31+ messages in thread
From: Pavel Hofman @ 2020-11-09 8:25 UTC (permalink / raw)
To: Ruslan Bilovol, balbi; +Cc: linux-usb, gschmottlach
Hi Ruslan, please excuse my stupid error, I really apologize...
Dne 09. 11. 20 v 9:24 Pavel Hofman napsal(a):
> Hi Bilovol,
>
> Dne 08. 11. 20 v 1:18 Ruslan Bilovol napsal(a):
>> This adds interface between userspace and feedback
>> endpoint to report real feedback frequency to the Host.
>>
>> Current implementation adds new userspace interface
>> ALSA mixer control "PCM Feedback Frequency Hz" (similar
>> to aloop driver's "PCM Rate Shift 100000" mixer control)
>>
>> We allow +/-20% deviation of nominal sampling frequency,
>> that usually is more than enough in real-world usecases
>>
>> Usage of this new control is easy to implement in
>> existing userspace tools like alsaloop from alsa-utils.
>>
>> Signed-off-by: Ruslan Bilovol <ruslan.bilovol@gmail.com>
>> ---
>> drivers/usb/gadget/function/f_uac2.c | 4 ++
>> drivers/usb/gadget/function/u_audio.c | 93 +++++++++++++++++++++++++++++++++++
>> drivers/usb/gadget/function/u_audio.h | 7 +++
>> 3 files changed, 104 insertions(+)
>
> Thanks a lot for the great implementation. IIUC the control element sets
> integer frequency in Hz. Often the clocks deviate by small fractions of
> Hz. Please have you considered the value to be e.g. in 100th of Hz for
> finer control of the samplerate? Similar to the PCM Rate Shift which has
> a step 100000th of the samplerate.
>
> Thanks a lot.
>
> Best regards,
>
> Pavel.
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 2/3] usb: gadget: f_uac2: add adaptive sync support for capture
2020-11-08 0:18 ` [PATCH 2/3] usb: gadget: f_uac2: add adaptive sync support for capture Ruslan Bilovol
@ 2020-11-11 9:18 ` Peter Chen
2020-11-12 22:39 ` Ruslan Bilovol
2020-11-26 11:13 ` Jerome Brunet
1 sibling, 1 reply; 31+ messages in thread
From: Peter Chen @ 2020-11-11 9:18 UTC (permalink / raw)
To: Ruslan Bilovol; +Cc: balbi, linux-usb, gschmottlach
On 20-11-08 02:18:30, Ruslan Bilovol wrote:
> Current f_uac2 USB OUT (aka 'capture') synchronization
> implements 'ASYNC' scenario which means USB Gadget has
> it's own freerunning clock and can update Host about
> real clock frequency through feedback endpoint so Host
> can align number of samples sent to the USB gadget to
> prevent overruns/underruns
>
> In case if Gadget can has no it's internal clock and
> can consume audio samples at any rate (for example,
> on the Gadget side someone records audio directly to
> a file, or audio samples are played through an
> external DAC as soon as they arrive), UAC2 spec
> suggests 'ADAPTIVE' synchronization type.
>
> Change UAC2 driver to make it configurable through
> additional 'c_sync' configfs file.
>
> Default remains 'asynchronous' with possibility to
> switch it to 'adaptive'
>
> Signed-off-by: Ruslan Bilovol <ruslan.bilovol@gmail.com>
> ---
> Documentation/ABI/testing/configfs-usb-gadget-uac2 | 1 +
> Documentation/usb/gadget-testing.rst | 1 +
> drivers/usb/gadget/function/f_uac2.c | 96 ++++++++++++++++++++--
> drivers/usb/gadget/function/u_uac2.h | 2 +
> 4 files changed, 91 insertions(+), 9 deletions(-)
>
> diff --git a/Documentation/ABI/testing/configfs-usb-gadget-uac2 b/Documentation/ABI/testing/configfs-usb-gadget-uac2
> index 2bfdd4e..4fbff96 100644
> --- a/Documentation/ABI/testing/configfs-usb-gadget-uac2
> +++ b/Documentation/ABI/testing/configfs-usb-gadget-uac2
> @@ -7,6 +7,7 @@ Description:
> c_chmask - capture channel mask
> c_srate - capture sampling rate
> c_ssize - capture sample size (bytes)
> + c_sync - capture synchronization type (async/adaptive)
Can't apply it based on the latest code, this file has changed.
> p_chmask - playback channel mask
> p_srate - playback sampling rate
> p_ssize - playback sample size (bytes)
> diff --git a/Documentation/usb/gadget-testing.rst b/Documentation/usb/gadget-testing.rst
> index 2eeb3e9..360a7ca 100644
> --- a/Documentation/usb/gadget-testing.rst
> +++ b/Documentation/usb/gadget-testing.rst
> @@ -728,6 +728,7 @@ The uac2 function provides these attributes in its function directory:
> c_chmask capture channel mask
> c_srate capture sampling rate
> c_ssize capture sample size (bytes)
> + c_sync capture synchronization type (async/adaptive)
> p_chmask playback channel mask
> p_srate playback sampling rate
> p_ssize playback sample size (bytes)
> diff --git a/drivers/usb/gadget/function/f_uac2.c b/drivers/usb/gadget/function/f_uac2.c
> index a57bf77..3187ad3 100644
> --- a/drivers/usb/gadget/function/f_uac2.c
> +++ b/drivers/usb/gadget/function/f_uac2.c
> @@ -41,6 +41,7 @@
>
> #define EPIN_EN(_opts) ((_opts)->p_chmask != 0)
> #define EPOUT_EN(_opts) ((_opts)->c_chmask != 0)
> +#define EPOUT_FBACK_IN_EN(_opts) ((_opts)->c_sync == USB_ENDPOINT_SYNC_ASYNC)
>
> struct f_uac2 {
> struct g_audio g_audio;
> @@ -237,7 +238,7 @@ enum {
> .bDescriptorType = USB_DT_INTERFACE,
>
> .bAlternateSetting = 1,
> - .bNumEndpoints = 2,
> + .bNumEndpoints = 1,
> .bInterfaceClass = USB_CLASS_AUDIO,
> .bInterfaceSubClass = USB_SUBCLASS_AUDIOSTREAMING,
> .bInterfaceProtocol = UAC_VERSION_2,
> @@ -270,7 +271,7 @@ enum {
> .bDescriptorType = USB_DT_ENDPOINT,
>
> .bEndpointAddress = USB_DIR_OUT,
> - .bmAttributes = USB_ENDPOINT_XFER_ISOC | USB_ENDPOINT_SYNC_ASYNC,
> + .bmAttributes = USB_ENDPOINT_XFER_ISOC,
> .wMaxPacketSize = cpu_to_le16(1023),
> .bInterval = 1,
> };
> @@ -279,7 +280,7 @@ enum {
> .bLength = USB_DT_ENDPOINT_SIZE,
> .bDescriptorType = USB_DT_ENDPOINT,
>
> - .bmAttributes = USB_ENDPOINT_XFER_ISOC | USB_ENDPOINT_SYNC_ASYNC,
> + .bmAttributes = USB_ENDPOINT_XFER_ISOC,
> .wMaxPacketSize = cpu_to_le16(1024),
> .bInterval = 1,
> };
> @@ -540,6 +541,19 @@ static void setup_descriptor(struct f_uac2_opts *opts)
> len += sizeof(io_out_ot_desc);
> ac_hdr_desc.wTotalLength = cpu_to_le16(len);
> iad_desc.bInterfaceCount++;
> +
> + if (EPOUT_FBACK_IN_EN(opts)) {
> + fs_epout_desc.bmAttributes |=
> + USB_ENDPOINT_SYNC_ASYNC;
> + hs_epout_desc.bmAttributes |=
> + USB_ENDPOINT_SYNC_ASYNC;
> + std_as_out_if1_desc.bNumEndpoints++;
> + } else {
> + fs_epout_desc.bmAttributes |=
> + USB_ENDPOINT_SYNC_ADAPTIVE;
> + hs_epout_desc.bmAttributes |=
> + USB_ENDPOINT_SYNC_ADAPTIVE;
> + }
> }
>
> i = 0;
> @@ -564,7 +578,8 @@ static void setup_descriptor(struct f_uac2_opts *opts)
> fs_audio_desc[i++] = USBDHDR(&as_out_fmt1_desc);
> fs_audio_desc[i++] = USBDHDR(&fs_epout_desc);
> fs_audio_desc[i++] = USBDHDR(&as_iso_out_desc);
> - fs_audio_desc[i++] = USBDHDR(&fs_epin_fback_desc);
> + if (EPOUT_FBACK_IN_EN(opts))
> + fs_audio_desc[i++] = USBDHDR(&fs_epin_fback_desc);
> }
> if (EPIN_EN(opts)) {
> fs_audio_desc[i++] = USBDHDR(&std_as_in_if0_desc);
> @@ -598,7 +613,8 @@ static void setup_descriptor(struct f_uac2_opts *opts)
> hs_audio_desc[i++] = USBDHDR(&as_out_fmt1_desc);
> hs_audio_desc[i++] = USBDHDR(&hs_epout_desc);
> hs_audio_desc[i++] = USBDHDR(&as_iso_out_desc);
> - hs_audio_desc[i++] = USBDHDR(&hs_epin_fback_desc);
> + if (EPOUT_FBACK_IN_EN(opts))
> + hs_audio_desc[i++] = USBDHDR(&hs_epin_fback_desc);
> }
> if (EPIN_EN(opts)) {
> hs_audio_desc[i++] = USBDHDR(&std_as_in_if0_desc);
> @@ -706,11 +722,14 @@ static void setup_descriptor(struct f_uac2_opts *opts)
> dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
> return -ENODEV;
> }
> - agdev->in_ep_fback = usb_ep_autoconfig(gadget,
> + if (EPOUT_FBACK_IN_EN(uac2_opts)) {
> + agdev->in_ep_fback = usb_ep_autoconfig(gadget,
> &fs_epin_fback_desc);
> - if (!agdev->in_ep_fback) {
> - dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
> - return -ENODEV;
> + if (!agdev->in_ep_fback) {
> + dev_err(dev, "%s:%d Error!\n",
> + __func__, __LINE__);
> + return -ENODEV;
> + }
> }
> }
>
> @@ -1057,11 +1076,68 @@ static void f_uac2_attr_release(struct config_item *item)
> \
> CONFIGFS_ATTR(f_uac2_opts_, name)
>
> +#define UAC2_ATTRIBUTE_SYNC(name) \
> +static ssize_t f_uac2_opts_##name##_show(struct config_item *item, \
> + char *page) \
> +{ \
> + struct f_uac2_opts *opts = to_f_uac2_opts(item); \
> + int result; \
> + char *str; \
> + \
> + mutex_lock(&opts->lock); \
> + switch (opts->name) { \
> + case USB_ENDPOINT_SYNC_ASYNC: \
> + str = "async"; \
> + break; \
> + case USB_ENDPOINT_SYNC_ADAPTIVE: \
> + str = "adaptive"; \
> + break; \
> + default: \
> + str = "unknown"; \
> + break; \
> + } \
> + result = sprintf(page, "%s\n", str); \
> + mutex_unlock(&opts->lock); \
> + \
> + return result; \
> +} \
> + \
> +static ssize_t f_uac2_opts_##name##_store(struct config_item *item, \
> + const char *page, size_t len) \
> +{ \
> + struct f_uac2_opts *opts = to_f_uac2_opts(item); \
> + int ret = 0; \
> + \
> + mutex_lock(&opts->lock); \
> + if (opts->refcnt) { \
> + ret = -EBUSY; \
> + goto end; \
> + } \
> + \
> + if (!strncmp(page, "async", 5)) \
> + opts->name = USB_ENDPOINT_SYNC_ASYNC; \
> + else if (!strncmp(page, "adaptive", 8)) \
> + opts->name = USB_ENDPOINT_SYNC_ADAPTIVE; \
> + else { \
> + ret = -EINVAL; \
> + goto end; \
> + } \
> + \
> + ret = len; \
> + \
> +end: \
> + mutex_unlock(&opts->lock); \
> + return ret; \
> +} \
> + \
> +CONFIGFS_ATTR(f_uac2_opts_, name)
> +
> UAC2_ATTRIBUTE(p_chmask);
> UAC2_ATTRIBUTE(p_srate);
> UAC2_ATTRIBUTE(p_ssize);
> UAC2_ATTRIBUTE(c_chmask);
> UAC2_ATTRIBUTE(c_srate);
> +UAC2_ATTRIBUTE_SYNC(c_sync);
> UAC2_ATTRIBUTE(c_ssize);
> UAC2_ATTRIBUTE(req_number);
>
> @@ -1072,6 +1148,7 @@ static void f_uac2_attr_release(struct config_item *item)
> &f_uac2_opts_attr_c_chmask,
> &f_uac2_opts_attr_c_srate,
> &f_uac2_opts_attr_c_ssize,
> + &f_uac2_opts_attr_c_sync,
> &f_uac2_opts_attr_req_number,
> NULL,
> };
> @@ -1110,6 +1187,7 @@ static struct usb_function_instance *afunc_alloc_inst(void)
> opts->c_chmask = UAC2_DEF_CCHMASK;
> opts->c_srate = UAC2_DEF_CSRATE;
> opts->c_ssize = UAC2_DEF_CSSIZE;
> + opts->c_sync = UAC2_DEF_CSYNC;
> opts->req_number = UAC2_DEF_REQ_NUM;
> return &opts->func_inst;
> }
> diff --git a/drivers/usb/gadget/function/u_uac2.h b/drivers/usb/gadget/function/u_uac2.h
> index b503571..13589c3 100644
> --- a/drivers/usb/gadget/function/u_uac2.h
> +++ b/drivers/usb/gadget/function/u_uac2.h
> @@ -21,6 +21,7 @@
> #define UAC2_DEF_CCHMASK 0x3
> #define UAC2_DEF_CSRATE 64000
> #define UAC2_DEF_CSSIZE 2
> +#define UAC2_DEF_CSYNC USB_ENDPOINT_SYNC_ASYNC
> #define UAC2_DEF_REQ_NUM 2
>
> struct f_uac2_opts {
> @@ -31,6 +32,7 @@ struct f_uac2_opts {
> int c_chmask;
> int c_srate;
> int c_ssize;
> + int c_sync;
> int req_number;
> bool bound;
>
> --
> 1.9.1
>
--
Thanks,
Peter Chen
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 1/3] usb: gadget: f_uac2/u_audio: add feedback endpoint support
2020-11-08 0:18 ` [PATCH 1/3] usb: gadget: f_uac2/u_audio: add " Ruslan Bilovol
@ 2020-11-11 9:26 ` Peter Chen
2020-11-12 22:41 ` Ruslan Bilovol
0 siblings, 1 reply; 31+ messages in thread
From: Peter Chen @ 2020-11-11 9:26 UTC (permalink / raw)
To: Ruslan Bilovol; +Cc: balbi, linux-usb, gschmottlach
On 20-11-08 02:18:29, Ruslan Bilovol wrote:
> As per USB and UAC2 specs, asynchronous audio sink endpoint
> requires explicit synchronization mechanism (Isochronous
> Feedback Endpoint)
>
> Implement feedback companion endpoint for ISO OUT endpoint
>
> This patch adds all required infrastructure and USB requests
> handling for feedback endpoint. Syncrhonization itself is
> still dummy (feedback ep always reports 'nomimal frequency'
> e.g. no adjustement is needed). This satisfies hosts that
> require feedback endpoint (like Win10) and poll it periodically
>
> Actual synchronization mechanism should be implemented
> separately
>
> Signed-off-by: Ruslan Bilovol <ruslan.bilovol@gmail.com>
> ---
> drivers/usb/gadget/function/f_uac2.c | 34 +++++++++-
> drivers/usb/gadget/function/u_audio.c | 124 +++++++++++++++++++++++++++++++++-
> drivers/usb/gadget/function/u_audio.h | 3 +
> 3 files changed, 158 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/gadget/function/f_uac2.c b/drivers/usb/gadget/function/f_uac2.c
> index fb9b875..a57bf77 100644
> --- a/drivers/usb/gadget/function/f_uac2.c
> +++ b/drivers/usb/gadget/function/f_uac2.c
> @@ -237,7 +237,7 @@ enum {
> .bDescriptorType = USB_DT_INTERFACE,
>
> .bAlternateSetting = 1,
> - .bNumEndpoints = 1,
> + .bNumEndpoints = 2,
> .bInterfaceClass = USB_CLASS_AUDIO,
> .bInterfaceSubClass = USB_SUBCLASS_AUDIOSTREAMING,
> .bInterfaceProtocol = UAC_VERSION_2,
> @@ -296,6 +296,27 @@ enum {
> .wLockDelay = 0,
> };
>
> +/* STD AS ISO IN Feedback Endpoint */
> +static struct usb_endpoint_descriptor fs_epin_fback_desc = {
> + .bLength = USB_DT_ENDPOINT_SIZE,
> + .bDescriptorType = USB_DT_ENDPOINT,
> +
> + .bEndpointAddress = USB_DIR_IN,
> + .bmAttributes = USB_ENDPOINT_XFER_ISOC | USB_ENDPOINT_USAGE_FEEDBACK,
> + .wMaxPacketSize = cpu_to_le16(3),
> + .bInterval = 1,
> +};
> +
> +static struct usb_endpoint_descriptor hs_epin_fback_desc = {
> + .bLength = USB_DT_ENDPOINT_SIZE,
> + .bDescriptorType = USB_DT_ENDPOINT,
> +
> + .bmAttributes = USB_ENDPOINT_XFER_ISOC | USB_ENDPOINT_USAGE_FEEDBACK,
> + .wMaxPacketSize = cpu_to_le16(4),
> + .bInterval = 4,
> +};
> +
> +
> /* Audio Streaming IN Interface - Alt0 */
> static struct usb_interface_descriptor std_as_in_if0_desc = {
> .bLength = sizeof std_as_in_if0_desc,
> @@ -392,6 +413,7 @@ enum {
> (struct usb_descriptor_header *)&as_out_fmt1_desc,
> (struct usb_descriptor_header *)&fs_epout_desc,
> (struct usb_descriptor_header *)&as_iso_out_desc,
> + (struct usb_descriptor_header *)&fs_epin_fback_desc,
>
> (struct usb_descriptor_header *)&std_as_in_if0_desc,
> (struct usb_descriptor_header *)&std_as_in_if1_desc,
> @@ -422,6 +444,7 @@ enum {
> (struct usb_descriptor_header *)&as_out_fmt1_desc,
> (struct usb_descriptor_header *)&hs_epout_desc,
> (struct usb_descriptor_header *)&as_iso_out_desc,
> + (struct usb_descriptor_header *)&hs_epin_fback_desc,
>
> (struct usb_descriptor_header *)&std_as_in_if0_desc,
> (struct usb_descriptor_header *)&std_as_in_if1_desc,
> @@ -541,6 +564,7 @@ static void setup_descriptor(struct f_uac2_opts *opts)
> fs_audio_desc[i++] = USBDHDR(&as_out_fmt1_desc);
> fs_audio_desc[i++] = USBDHDR(&fs_epout_desc);
> fs_audio_desc[i++] = USBDHDR(&as_iso_out_desc);
> + fs_audio_desc[i++] = USBDHDR(&fs_epin_fback_desc);
> }
> if (EPIN_EN(opts)) {
> fs_audio_desc[i++] = USBDHDR(&std_as_in_if0_desc);
> @@ -574,6 +598,7 @@ static void setup_descriptor(struct f_uac2_opts *opts)
> hs_audio_desc[i++] = USBDHDR(&as_out_fmt1_desc);
> hs_audio_desc[i++] = USBDHDR(&hs_epout_desc);
> hs_audio_desc[i++] = USBDHDR(&as_iso_out_desc);
> + hs_audio_desc[i++] = USBDHDR(&hs_epin_fback_desc);
> }
> if (EPIN_EN(opts)) {
> hs_audio_desc[i++] = USBDHDR(&std_as_in_if0_desc);
> @@ -681,6 +706,12 @@ static void setup_descriptor(struct f_uac2_opts *opts)
> dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
> return -ENODEV;
> }
> + agdev->in_ep_fback = usb_ep_autoconfig(gadget,
> + &fs_epin_fback_desc);
> + if (!agdev->in_ep_fback) {
> + dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
> + return -ENODEV;
> + }
> }
>
> if (EPIN_EN(uac2_opts)) {
> @@ -699,6 +730,7 @@ static void setup_descriptor(struct f_uac2_opts *opts)
> le16_to_cpu(hs_epout_desc.wMaxPacketSize));
>
> hs_epout_desc.bEndpointAddress = fs_epout_desc.bEndpointAddress;
> + hs_epin_fback_desc.bEndpointAddress = fs_epin_fback_desc.bEndpointAddress;
> hs_epin_desc.bEndpointAddress = fs_epin_desc.bEndpointAddress;
>
> setup_descriptor(uac2_opts);
> diff --git a/drivers/usb/gadget/function/u_audio.c b/drivers/usb/gadget/function/u_audio.c
> index e6d32c5..33c9288 100644
> --- a/drivers/usb/gadget/function/u_audio.c
> +++ b/drivers/usb/gadget/function/u_audio.c
> @@ -43,6 +43,10 @@ struct uac_rtd_params {
> unsigned int max_psize; /* MaxPacketSize of endpoint */
> struct uac_req *ureq;
>
> + struct usb_request *req_fback; /* Feedback endpoint request */
> + unsigned int ffback; /* Real frequency reported by feedback endpoint */
> + bool fb_ep_enabled; /* if the ep is enabled */
> +
> spinlock_t lock;
> };
>
> @@ -76,6 +80,35 @@ struct snd_uac_chip {
> .periods_min = MIN_PERIODS,
> };
>
> +static void u_audio_set_fback_frequency(enum usb_device_speed speed,
> + unsigned int freq, void *buf)
> +{
> + u32 ff = 0;
> +
> + if (speed == USB_SPEED_FULL) {
> + /*
> + * Full-speed feedback endpoints report frequency
> + * in samples/microframe
> + * Format is encoded in Q10.10 left-justified in the 24 bits,
> + * so that it has a Q10.14 format.
> + */
> + ff = DIV_ROUND_UP((freq << 14), 1000);
> + } else {
> + /*
> + * High-speed feedback endpoints report frequency
> + * in samples/microframe.
> + * Format is encoded in Q12.13 fitted into four bytes so that
> + * the binary point is located between the second and the third
> + * byte fromat (that is Q16.16)
> + *
> + * Prevent integer overflow by calculating in Q12.13 fromat and
> + * then shifting to Q16.16
> + */
> + ff = DIV_ROUND_UP((freq << 13), (8*1000)) << 3;
> + }
> + *(__le32 *)buf = cpu_to_le32(ff);
> +}
> +
> static void u_audio_iso_complete(struct usb_ep *ep, struct usb_request *req)
> {
> unsigned int pending;
> @@ -182,6 +215,36 @@ static void u_audio_iso_complete(struct usb_ep *ep, struct usb_request *req)
> dev_err(uac->card->dev, "%d Error!\n", __LINE__);
> }
>
> +static void u_audio_iso_fback_complete(struct usb_ep *ep,
> + struct usb_request *req)
> +{
> + struct uac_rtd_params *prm = req->context;
> + struct snd_uac_chip *uac = prm->uac;
> + struct g_audio *audio_dev = uac->audio_dev;
> + int status = req->status;
> + unsigned long flags;
> +
> + /* i/f shutting down */
> + if (!prm->fb_ep_enabled || req->status == -ESHUTDOWN)
> + return;
> +
> + /*
> + * We can't really do much about bad xfers.
> + * Afterall, the ISOCH xfers could fail legitimately.
> + */
> + if (status)
> + pr_debug("%s: iso_complete status(%d) %d/%d\n",
> + __func__, status, req->actual, req->length);
> +
> + spin_lock_irqsave(&prm->lock, flags);
> + u_audio_set_fback_frequency(audio_dev->gadget->speed,
> + prm->ffback, req->buf);
> + spin_unlock_irqrestore(&prm->lock, flags);
> +
> + if (usb_ep_queue(ep, req, GFP_ATOMIC))
> + dev_err(uac->card->dev, "%d Error!\n", __LINE__);
> +}
> +
> static int uac_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
> {
> struct snd_uac_chip *uac = snd_pcm_substream_chip(substream);
> @@ -346,14 +409,33 @@ static inline void free_ep(struct uac_rtd_params *prm, struct usb_ep *ep)
> dev_err(uac->card->dev, "%s:%d Error!\n", __func__, __LINE__);
> }
>
> +static inline void free_ep_fback(struct uac_rtd_params *prm, struct usb_ep *ep)
> +{
> + struct snd_uac_chip *uac = prm->uac;
> +
> + if (!prm->fb_ep_enabled)
> + return;
> +
> + prm->fb_ep_enabled = false;
> +
> + if (prm->req_fback) {
> + usb_ep_dequeue(ep, prm->req_fback);
> + kfree(prm->req_fback->buf);
You may need to free buf at completion, Jack Pham found a use-after-free
bug at dwc3 controller due to async execution for usb_ep_dequeue.
https://www.spinics.net/lists/linux-usb/msg203782.html
> + usb_ep_free_request(ep, prm->req_fback);
> + prm->req_fback = NULL;
> + }
> +
> + if (usb_ep_disable(ep))
> + dev_err(uac->card->dev, "%s:%d Error!\n", __func__, __LINE__);
> +}
>
> int u_audio_start_capture(struct g_audio *audio_dev)
> {
> struct snd_uac_chip *uac = audio_dev->uac;
> struct usb_gadget *gadget = audio_dev->gadget;
> struct device *dev = &gadget->dev;
> - struct usb_request *req;
> - struct usb_ep *ep;
> + struct usb_request *req, *req_fback;
> + struct usb_ep *ep, *ep_fback;
> struct uac_rtd_params *prm;
> struct uac_params *params = &audio_dev->params;
> int req_len, i;
> @@ -386,6 +468,42 @@ int u_audio_start_capture(struct g_audio *audio_dev)
> dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
> }
>
> + ep_fback = audio_dev->in_ep_fback;
> + if (!ep_fback)
> + return 0;
> +
> + /* Setup feedback endpoint */
> + config_ep_by_speed(gadget, &audio_dev->func, ep_fback);
> + prm->fb_ep_enabled = true;
> + usb_ep_enable(ep_fback);
> + req_len = ep_fback->maxpacket;
> +
> + req_fback = usb_ep_alloc_request(ep_fback, GFP_ATOMIC);
> + if (req_fback == NULL)
> + return -ENOMEM;
> +
> + prm->req_fback = req_fback;
> + req_fback->zero = 0;
> + req_fback->context = prm;
> + req_fback->length = req_len;
> + req_fback->complete = u_audio_iso_fback_complete;
> +
> + req_fback->buf = kzalloc(req_len, GFP_ATOMIC);
> + if (!req_fback->buf)
> + return -ENOMEM;
> +
> + /*
> + * Configure the feedback endpoint's reported frequency.
> + * Always start with original frequency since its deviation can't
> + * be meauserd at start of playback
%s/meauserd/measured
> + */
> + prm->ffback = params->c_srate;
> + u_audio_set_fback_frequency(audio_dev->gadget->speed,
> + prm->ffback, req_fback->buf);
> +
> + if (usb_ep_queue(ep_fback, req_fback, GFP_ATOMIC))
> + dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
> +
> return 0;
> }
> EXPORT_SYMBOL_GPL(u_audio_start_capture);
> @@ -394,6 +512,8 @@ void u_audio_stop_capture(struct g_audio *audio_dev)
> {
> struct snd_uac_chip *uac = audio_dev->uac;
>
> + 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);
> }
> EXPORT_SYMBOL_GPL(u_audio_stop_capture);
> diff --git a/drivers/usb/gadget/function/u_audio.h b/drivers/usb/gadget/function/u_audio.h
> index 5ea6b86..53e6baf 100644
> --- a/drivers/usb/gadget/function/u_audio.h
> +++ b/drivers/usb/gadget/function/u_audio.h
> @@ -30,7 +30,10 @@ struct g_audio {
> struct usb_gadget *gadget;
>
> struct usb_ep *in_ep;
> +
> struct usb_ep *out_ep;
> + /* feedback IN endpoint corresponding to out_ep */
> + struct usb_ep *in_ep_fback;
>
> /* Max packet size for all in_ep possible speeds */
> unsigned int in_ep_maxpsize;
> --
> 1.9.1
>
--
Thanks,
Peter Chen
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 0/3] UAC2 Gadget: feedback endpoint support
2020-11-08 0:18 [PATCH 0/3] UAC2 Gadget: feedback endpoint support Ruslan Bilovol
` (2 preceding siblings ...)
2020-11-08 0:18 ` [PATCH 3/3] usb: gadget: u_audio: add real feedback implementation Ruslan Bilovol
@ 2020-11-11 9:30 ` Peter Chen
2020-11-12 23:20 ` Ruslan Bilovol
3 siblings, 1 reply; 31+ messages in thread
From: Peter Chen @ 2020-11-11 9:30 UTC (permalink / raw)
To: Ruslan Bilovol; +Cc: balbi, linux-usb, gschmottlach
On 20-11-08 02:18:28, Ruslan Bilovol wrote:
> Current UAC2 gadget implements capture/sync paths
> as two USB ISO ASYNC endpoints (IN and OUT).
>
> This violates USB spec which says that ISO ASYNC OUT endpoint
> should have feedback companion endpoint.
> See USB2.0 spec "5.12.4.1 Synchronization Type": asynchronous
> sink provides explicit feedback (isochronous pipe).
> Interesting that for ISO ASYNC *IN* endpoint respective
> feedback isn't required since source provides implicit
> feedforward (data stream).
>
> While it's not an issue if UAC2 Gadget is connected to
> Linux host (Linux ignores missing feedback endpoint),
> with other hosts like Windows or MacOS the UAC2 Gadget
> isn't enumerated due to missing feedback endpoint.
>
> This patch series adds feedback endpoint support to
> UAC2 function, new control to UAC2 mixer which can
> be used by userspace tools (like alsaloop from alsa-utils)
> for updating feedback frequency reported to the host.
> This is useful for usecases when UAC2 Gadget's audio
> samples are played to another codec or audio card
> with its own internal freerunning clock so host can
> be notified that more/less samples are required.
>
> The alsaloop tool requires some (relatively small)
> modifications in order to start support driving
> feedback frequency through UAC2 mixer control.
> That change will be sent as a separate patch
> to ALSA community.
>
> Also added ability to switch ISO ASYNC OUT endpoint into
> adaptive endpoint which doesn't require feedback endpoint
> (as per USB spec).
>
> Ruslan Bilovol (3):
> usb: gadget: f_uac2/u_audio: add feedback endpoint support
> usb: gadget: f_uac2: add adaptive sync support for capture
> usb: gadget: u_audio: add real feedback implementation
Hi Ruslan,
I applied your patches, but WIN10 still can't recognize it well.
The UAC1 is OK for WIN10 with the below same configuration.
Any debug information you would like to know to check it?
if [ "$FUNC" == "uac2" ]; then
mkdir functions/$FUNC".0"
echo 2 > functions/$FUNC".0"/p_ssize
echo 48000 > functions/$FUNC".0"/p_srate
echo 3 > functions/$FUNC".0"/p_chmask
echo 2 > functions/$FUNC".0"/c_ssize
echo 48000 > functions/$FUNC".0"/c_srate
echo 3 > functions/$FUNC".0"/c_chmask
#echo 4 > functions/$FUNC".0"/req_number
ln -s functions/$FUNC".0" configs/c.1
echo high-speed > /sys/kernel/config/usb_gadget/g1/max_speed
fi
--
Thanks,
Peter Chen
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 3/3] usb: gadget: u_audio: add real feedback implementation
2020-11-09 8:24 ` Pavel Hofman
2020-11-09 8:25 ` Pavel Hofman
@ 2020-11-12 11:26 ` Pavel Hofman
1 sibling, 0 replies; 31+ messages in thread
From: Pavel Hofman @ 2020-11-12 11:26 UTC (permalink / raw)
To: Ruslan Bilovol, balbi; +Cc: linux-usb, gschmottlach
Dne 09. 11. 20 v 9:24 Pavel Hofman napsal(a):
> Hi Ruslan,
>
> Dne 08. 11. 20 v 1:18 Ruslan Bilovol napsal(a):
>> This adds interface between userspace and feedback
>> endpoint to report real feedback frequency to the Host.
>>
>> Current implementation adds new userspace interface
>> ALSA mixer control "PCM Feedback Frequency Hz" (similar
>> to aloop driver's "PCM Rate Shift 100000" mixer control)
>>
>> We allow +/-20% deviation of nominal sampling frequency,
>> that usually is more than enough in real-world usecases
>>
>> Usage of this new control is easy to implement in
>> existing userspace tools like alsaloop from alsa-utils.
>>
>> Signed-off-by: Ruslan Bilovol <ruslan.bilovol@gmail.com>
>> ---
>> drivers/usb/gadget/function/f_uac2.c | 4 ++
>> drivers/usb/gadget/function/u_audio.c | 93 +++++++++++++++++++++++++++++++++++
>> drivers/usb/gadget/function/u_audio.h | 7 +++
>> 3 files changed, 104 insertions(+)
>
> Thanks a lot for the great implementation. IIUC the control element sets
> integer frequency in Hz. Often the clocks deviate by small fractions of
> Hz. Please have you considered the value to be e.g. in 100th of Hz for
> finer control of the samplerate? Similar to the PCM Rate Shift which has
> a step 100000th of the samplerate.
>
My stupid, I did not realize that one Hz in the samplerate value is
basically the same order of magnitude as the 100,000ths in the "PCM Rate
Shift 100000" mixer control. Sorry for disturbing.
Thanks,
Pavel.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 2/3] usb: gadget: f_uac2: add adaptive sync support for capture
2020-11-11 9:18 ` Peter Chen
@ 2020-11-12 22:39 ` Ruslan Bilovol
0 siblings, 0 replies; 31+ messages in thread
From: Ruslan Bilovol @ 2020-11-12 22:39 UTC (permalink / raw)
To: Peter Chen; +Cc: balbi, linux-usb, gschmottlach
On Wed, Nov 11, 2020 at 11:18 AM Peter Chen <peter.chen@nxp.com> wrote:
>
> On 20-11-08 02:18:30, Ruslan Bilovol wrote:
> > Current f_uac2 USB OUT (aka 'capture') synchronization
> > implements 'ASYNC' scenario which means USB Gadget has
> > it's own freerunning clock and can update Host about
> > real clock frequency through feedback endpoint so Host
> > can align number of samples sent to the USB gadget to
> > prevent overruns/underruns
> >
> > In case if Gadget can has no it's internal clock and
> > can consume audio samples at any rate (for example,
> > on the Gadget side someone records audio directly to
> > a file, or audio samples are played through an
> > external DAC as soon as they arrive), UAC2 spec
> > suggests 'ADAPTIVE' synchronization type.
> >
> > Change UAC2 driver to make it configurable through
> > additional 'c_sync' configfs file.
> >
> > Default remains 'asynchronous' with possibility to
> > switch it to 'adaptive'
> >
> > Signed-off-by: Ruslan Bilovol <ruslan.bilovol@gmail.com>
> > ---
> > Documentation/ABI/testing/configfs-usb-gadget-uac2 | 1 +
> > Documentation/usb/gadget-testing.rst | 1 +
> > drivers/usb/gadget/function/f_uac2.c | 96 ++++++++++++++++++++--
> > drivers/usb/gadget/function/u_uac2.h | 2 +
> > 4 files changed, 91 insertions(+), 9 deletions(-)
> >
> > diff --git a/Documentation/ABI/testing/configfs-usb-gadget-uac2 b/Documentation/ABI/testing/configfs-usb-gadget-uac2
> > index 2bfdd4e..4fbff96 100644
> > --- a/Documentation/ABI/testing/configfs-usb-gadget-uac2
> > +++ b/Documentation/ABI/testing/configfs-usb-gadget-uac2
> > @@ -7,6 +7,7 @@ Description:
> > c_chmask - capture channel mask
> > c_srate - capture sampling rate
> > c_ssize - capture sample size (bytes)
> > + c_sync - capture synchronization type (async/adaptive)
>
> Can't apply it based on the latest code, this file has changed.
This patch set is based on Felipe's -next branch which seems
to be a little bit outdated.
Thanks,
Ruslan
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 1/3] usb: gadget: f_uac2/u_audio: add feedback endpoint support
2020-11-11 9:26 ` Peter Chen
@ 2020-11-12 22:41 ` Ruslan Bilovol
0 siblings, 0 replies; 31+ messages in thread
From: Ruslan Bilovol @ 2020-11-12 22:41 UTC (permalink / raw)
To: Peter Chen; +Cc: balbi, linux-usb, gschmottlach
On Wed, Nov 11, 2020 at 11:26 AM Peter Chen <peter.chen@nxp.com> wrote:
>
> On 20-11-08 02:18:29, Ruslan Bilovol wrote:
> > As per USB and UAC2 specs, asynchronous audio sink endpoint
> > requires explicit synchronization mechanism (Isochronous
> > Feedback Endpoint)
> >
> > Implement feedback companion endpoint for ISO OUT endpoint
> >
> > This patch adds all required infrastructure and USB requests
> > handling for feedback endpoint. Syncrhonization itself is
> > still dummy (feedback ep always reports 'nomimal frequency'
> > e.g. no adjustement is needed). This satisfies hosts that
> > require feedback endpoint (like Win10) and poll it periodically
> >
> > Actual synchronization mechanism should be implemented
> > separately
> >
> > Signed-off-by: Ruslan Bilovol <ruslan.bilovol@gmail.com>
> > ---
> > drivers/usb/gadget/function/f_uac2.c | 34 +++++++++-
> > drivers/usb/gadget/function/u_audio.c | 124 +++++++++++++++++++++++++++++++++-
> > drivers/usb/gadget/function/u_audio.h | 3 +
> > 3 files changed, 158 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/usb/gadget/function/f_uac2.c b/drivers/usb/gadget/function/f_uac2.c
> > index fb9b875..a57bf77 100644
> > --- a/drivers/usb/gadget/function/f_uac2.c
> > +++ b/drivers/usb/gadget/function/f_uac2.c
> > @@ -237,7 +237,7 @@ enum {
> > .bDescriptorType = USB_DT_INTERFACE,
> >
> > .bAlternateSetting = 1,
> > - .bNumEndpoints = 1,
> > + .bNumEndpoints = 2,
> > .bInterfaceClass = USB_CLASS_AUDIO,
> > .bInterfaceSubClass = USB_SUBCLASS_AUDIOSTREAMING,
> > .bInterfaceProtocol = UAC_VERSION_2,
> > @@ -296,6 +296,27 @@ enum {
> > .wLockDelay = 0,
> > };
> >
> > +/* STD AS ISO IN Feedback Endpoint */
> > +static struct usb_endpoint_descriptor fs_epin_fback_desc = {
> > + .bLength = USB_DT_ENDPOINT_SIZE,
> > + .bDescriptorType = USB_DT_ENDPOINT,
> > +
> > + .bEndpointAddress = USB_DIR_IN,
> > + .bmAttributes = USB_ENDPOINT_XFER_ISOC | USB_ENDPOINT_USAGE_FEEDBACK,
> > + .wMaxPacketSize = cpu_to_le16(3),
> > + .bInterval = 1,
> > +};
> > +
> > +static struct usb_endpoint_descriptor hs_epin_fback_desc = {
> > + .bLength = USB_DT_ENDPOINT_SIZE,
> > + .bDescriptorType = USB_DT_ENDPOINT,
> > +
> > + .bmAttributes = USB_ENDPOINT_XFER_ISOC | USB_ENDPOINT_USAGE_FEEDBACK,
> > + .wMaxPacketSize = cpu_to_le16(4),
> > + .bInterval = 4,
> > +};
> > +
> > +
> > /* Audio Streaming IN Interface - Alt0 */
> > static struct usb_interface_descriptor std_as_in_if0_desc = {
> > .bLength = sizeof std_as_in_if0_desc,
> > @@ -392,6 +413,7 @@ enum {
> > (struct usb_descriptor_header *)&as_out_fmt1_desc,
> > (struct usb_descriptor_header *)&fs_epout_desc,
> > (struct usb_descriptor_header *)&as_iso_out_desc,
> > + (struct usb_descriptor_header *)&fs_epin_fback_desc,
> >
> > (struct usb_descriptor_header *)&std_as_in_if0_desc,
> > (struct usb_descriptor_header *)&std_as_in_if1_desc,
> > @@ -422,6 +444,7 @@ enum {
> > (struct usb_descriptor_header *)&as_out_fmt1_desc,
> > (struct usb_descriptor_header *)&hs_epout_desc,
> > (struct usb_descriptor_header *)&as_iso_out_desc,
> > + (struct usb_descriptor_header *)&hs_epin_fback_desc,
> >
> > (struct usb_descriptor_header *)&std_as_in_if0_desc,
> > (struct usb_descriptor_header *)&std_as_in_if1_desc,
> > @@ -541,6 +564,7 @@ static void setup_descriptor(struct f_uac2_opts *opts)
> > fs_audio_desc[i++] = USBDHDR(&as_out_fmt1_desc);
> > fs_audio_desc[i++] = USBDHDR(&fs_epout_desc);
> > fs_audio_desc[i++] = USBDHDR(&as_iso_out_desc);
> > + fs_audio_desc[i++] = USBDHDR(&fs_epin_fback_desc);
> > }
> > if (EPIN_EN(opts)) {
> > fs_audio_desc[i++] = USBDHDR(&std_as_in_if0_desc);
> > @@ -574,6 +598,7 @@ static void setup_descriptor(struct f_uac2_opts *opts)
> > hs_audio_desc[i++] = USBDHDR(&as_out_fmt1_desc);
> > hs_audio_desc[i++] = USBDHDR(&hs_epout_desc);
> > hs_audio_desc[i++] = USBDHDR(&as_iso_out_desc);
> > + hs_audio_desc[i++] = USBDHDR(&hs_epin_fback_desc);
> > }
> > if (EPIN_EN(opts)) {
> > hs_audio_desc[i++] = USBDHDR(&std_as_in_if0_desc);
> > @@ -681,6 +706,12 @@ static void setup_descriptor(struct f_uac2_opts *opts)
> > dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
> > return -ENODEV;
> > }
> > + agdev->in_ep_fback = usb_ep_autoconfig(gadget,
> > + &fs_epin_fback_desc);
> > + if (!agdev->in_ep_fback) {
> > + dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
> > + return -ENODEV;
> > + }
> > }
> >
> > if (EPIN_EN(uac2_opts)) {
> > @@ -699,6 +730,7 @@ static void setup_descriptor(struct f_uac2_opts *opts)
> > le16_to_cpu(hs_epout_desc.wMaxPacketSize));
> >
> > hs_epout_desc.bEndpointAddress = fs_epout_desc.bEndpointAddress;
> > + hs_epin_fback_desc.bEndpointAddress = fs_epin_fback_desc.bEndpointAddress;
> > hs_epin_desc.bEndpointAddress = fs_epin_desc.bEndpointAddress;
> >
> > setup_descriptor(uac2_opts);
> > diff --git a/drivers/usb/gadget/function/u_audio.c b/drivers/usb/gadget/function/u_audio.c
> > index e6d32c5..33c9288 100644
> > --- a/drivers/usb/gadget/function/u_audio.c
> > +++ b/drivers/usb/gadget/function/u_audio.c
> > @@ -43,6 +43,10 @@ struct uac_rtd_params {
> > unsigned int max_psize; /* MaxPacketSize of endpoint */
> > struct uac_req *ureq;
> >
> > + struct usb_request *req_fback; /* Feedback endpoint request */
> > + unsigned int ffback; /* Real frequency reported by feedback endpoint */
> > + bool fb_ep_enabled; /* if the ep is enabled */
> > +
> > spinlock_t lock;
> > };
> >
> > @@ -76,6 +80,35 @@ struct snd_uac_chip {
> > .periods_min = MIN_PERIODS,
> > };
> >
> > +static void u_audio_set_fback_frequency(enum usb_device_speed speed,
> > + unsigned int freq, void *buf)
> > +{
> > + u32 ff = 0;
> > +
> > + if (speed == USB_SPEED_FULL) {
> > + /*
> > + * Full-speed feedback endpoints report frequency
> > + * in samples/microframe
> > + * Format is encoded in Q10.10 left-justified in the 24 bits,
> > + * so that it has a Q10.14 format.
> > + */
> > + ff = DIV_ROUND_UP((freq << 14), 1000);
> > + } else {
> > + /*
> > + * High-speed feedback endpoints report frequency
> > + * in samples/microframe.
> > + * Format is encoded in Q12.13 fitted into four bytes so that
> > + * the binary point is located between the second and the third
> > + * byte fromat (that is Q16.16)
> > + *
> > + * Prevent integer overflow by calculating in Q12.13 fromat and
> > + * then shifting to Q16.16
> > + */
> > + ff = DIV_ROUND_UP((freq << 13), (8*1000)) << 3;
> > + }
> > + *(__le32 *)buf = cpu_to_le32(ff);
> > +}
> > +
> > static void u_audio_iso_complete(struct usb_ep *ep, struct usb_request *req)
> > {
> > unsigned int pending;
> > @@ -182,6 +215,36 @@ static void u_audio_iso_complete(struct usb_ep *ep, struct usb_request *req)
> > dev_err(uac->card->dev, "%d Error!\n", __LINE__);
> > }
> >
> > +static void u_audio_iso_fback_complete(struct usb_ep *ep,
> > + struct usb_request *req)
> > +{
> > + struct uac_rtd_params *prm = req->context;
> > + struct snd_uac_chip *uac = prm->uac;
> > + struct g_audio *audio_dev = uac->audio_dev;
> > + int status = req->status;
> > + unsigned long flags;
> > +
> > + /* i/f shutting down */
> > + if (!prm->fb_ep_enabled || req->status == -ESHUTDOWN)
> > + return;
> > +
> > + /*
> > + * We can't really do much about bad xfers.
> > + * Afterall, the ISOCH xfers could fail legitimately.
> > + */
> > + if (status)
> > + pr_debug("%s: iso_complete status(%d) %d/%d\n",
> > + __func__, status, req->actual, req->length);
> > +
> > + spin_lock_irqsave(&prm->lock, flags);
> > + u_audio_set_fback_frequency(audio_dev->gadget->speed,
> > + prm->ffback, req->buf);
> > + spin_unlock_irqrestore(&prm->lock, flags);
> > +
> > + if (usb_ep_queue(ep, req, GFP_ATOMIC))
> > + dev_err(uac->card->dev, "%d Error!\n", __LINE__);
> > +}
> > +
> > static int uac_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
> > {
> > struct snd_uac_chip *uac = snd_pcm_substream_chip(substream);
> > @@ -346,14 +409,33 @@ static inline void free_ep(struct uac_rtd_params *prm, struct usb_ep *ep)
> > dev_err(uac->card->dev, "%s:%d Error!\n", __func__, __LINE__);
> > }
> >
> > +static inline void free_ep_fback(struct uac_rtd_params *prm, struct usb_ep *ep)
> > +{
> > + struct snd_uac_chip *uac = prm->uac;
> > +
> > + if (!prm->fb_ep_enabled)
> > + return;
> > +
> > + prm->fb_ep_enabled = false;
> > +
> > + if (prm->req_fback) {
> > + usb_ep_dequeue(ep, prm->req_fback);
> > + kfree(prm->req_fback->buf);
>
> You may need to free buf at completion, Jack Pham found a use-after-free
> bug at dwc3 controller due to async execution for usb_ep_dequeue.
>
> https://www.spinics.net/lists/linux-usb/msg203782.html
Will review and retest with that change
>
> > + usb_ep_free_request(ep, prm->req_fback);
> > + prm->req_fback = NULL;
> > + }
> > +
> > + if (usb_ep_disable(ep))
> > + dev_err(uac->card->dev, "%s:%d Error!\n", __func__, __LINE__);
> > +}
> >
> > int u_audio_start_capture(struct g_audio *audio_dev)
> > {
> > struct snd_uac_chip *uac = audio_dev->uac;
> > struct usb_gadget *gadget = audio_dev->gadget;
> > struct device *dev = &gadget->dev;
> > - struct usb_request *req;
> > - struct usb_ep *ep;
> > + struct usb_request *req, *req_fback;
> > + struct usb_ep *ep, *ep_fback;
> > struct uac_rtd_params *prm;
> > struct uac_params *params = &audio_dev->params;
> > int req_len, i;
> > @@ -386,6 +468,42 @@ int u_audio_start_capture(struct g_audio *audio_dev)
> > dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
> > }
> >
> > + ep_fback = audio_dev->in_ep_fback;
> > + if (!ep_fback)
> > + return 0;
> > +
> > + /* Setup feedback endpoint */
> > + config_ep_by_speed(gadget, &audio_dev->func, ep_fback);
> > + prm->fb_ep_enabled = true;
> > + usb_ep_enable(ep_fback);
> > + req_len = ep_fback->maxpacket;
> > +
> > + req_fback = usb_ep_alloc_request(ep_fback, GFP_ATOMIC);
> > + if (req_fback == NULL)
> > + return -ENOMEM;
> > +
> > + prm->req_fback = req_fback;
> > + req_fback->zero = 0;
> > + req_fback->context = prm;
> > + req_fback->length = req_len;
> > + req_fback->complete = u_audio_iso_fback_complete;
> > +
> > + req_fback->buf = kzalloc(req_len, GFP_ATOMIC);
> > + if (!req_fback->buf)
> > + return -ENOMEM;
> > +
> > + /*
> > + * Configure the feedback endpoint's reported frequency.
> > + * Always start with original frequency since its deviation can't
> > + * be meauserd at start of playback
>
> %s/meauserd/measured
Good catch!
Thanks,
Ruslan
>
> > + */
> > + prm->ffback = params->c_srate;
> > + u_audio_set_fback_frequency(audio_dev->gadget->speed,
> > + prm->ffback, req_fback->buf);
> > +
> > + if (usb_ep_queue(ep_fback, req_fback, GFP_ATOMIC))
> > + dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
> > +
> > return 0;
> > }
> > EXPORT_SYMBOL_GPL(u_audio_start_capture);
> > @@ -394,6 +512,8 @@ void u_audio_stop_capture(struct g_audio *audio_dev)
> > {
> > struct snd_uac_chip *uac = audio_dev->uac;
> >
> > + 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);
> > }
> > EXPORT_SYMBOL_GPL(u_audio_stop_capture);
> > diff --git a/drivers/usb/gadget/function/u_audio.h b/drivers/usb/gadget/function/u_audio.h
> > index 5ea6b86..53e6baf 100644
> > --- a/drivers/usb/gadget/function/u_audio.h
> > +++ b/drivers/usb/gadget/function/u_audio.h
> > @@ -30,7 +30,10 @@ struct g_audio {
> > struct usb_gadget *gadget;
> >
> > struct usb_ep *in_ep;
> > +
> > struct usb_ep *out_ep;
> > + /* feedback IN endpoint corresponding to out_ep */
> > + struct usb_ep *in_ep_fback;
> >
> > /* Max packet size for all in_ep possible speeds */
> > unsigned int in_ep_maxpsize;
> > --
> > 1.9.1
> >
>
> --
>
> Thanks,
> Peter Chen
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 0/3] UAC2 Gadget: feedback endpoint support
2020-11-11 9:30 ` [PATCH 0/3] UAC2 Gadget: feedback endpoint support Peter Chen
@ 2020-11-12 23:20 ` Ruslan Bilovol
2020-11-13 15:35 ` Glenn Schmottlach
0 siblings, 1 reply; 31+ messages in thread
From: Ruslan Bilovol @ 2020-11-12 23:20 UTC (permalink / raw)
To: Peter Chen; +Cc: balbi, linux-usb, gschmottlach
On Wed, Nov 11, 2020 at 11:30 AM Peter Chen <peter.chen@nxp.com> wrote:
>
> On 20-11-08 02:18:28, Ruslan Bilovol wrote:
> > Current UAC2 gadget implements capture/sync paths
> > as two USB ISO ASYNC endpoints (IN and OUT).
> >
> > This violates USB spec which says that ISO ASYNC OUT endpoint
> > should have feedback companion endpoint.
> > See USB2.0 spec "5.12.4.1 Synchronization Type": asynchronous
> > sink provides explicit feedback (isochronous pipe).
> > Interesting that for ISO ASYNC *IN* endpoint respective
> > feedback isn't required since source provides implicit
> > feedforward (data stream).
> >
> > While it's not an issue if UAC2 Gadget is connected to
> > Linux host (Linux ignores missing feedback endpoint),
> > with other hosts like Windows or MacOS the UAC2 Gadget
> > isn't enumerated due to missing feedback endpoint.
> >
> > This patch series adds feedback endpoint support to
> > UAC2 function, new control to UAC2 mixer which can
> > be used by userspace tools (like alsaloop from alsa-utils)
> > for updating feedback frequency reported to the host.
> > This is useful for usecases when UAC2 Gadget's audio
> > samples are played to another codec or audio card
> > with its own internal freerunning clock so host can
> > be notified that more/less samples are required.
> >
> > The alsaloop tool requires some (relatively small)
> > modifications in order to start support driving
> > feedback frequency through UAC2 mixer control.
> > That change will be sent as a separate patch
> > to ALSA community.
> >
> > Also added ability to switch ISO ASYNC OUT endpoint into
> > adaptive endpoint which doesn't require feedback endpoint
> > (as per USB spec).
> >
> > Ruslan Bilovol (3):
> > usb: gadget: f_uac2/u_audio: add feedback endpoint support
> > usb: gadget: f_uac2: add adaptive sync support for capture
> > usb: gadget: u_audio: add real feedback implementation
>
> Hi Ruslan,
>
> I applied your patches, but WIN10 still can't recognize it well.
> The UAC1 is OK for WIN10 with the below same configuration.
> Any debug information you would like to know to check it?
>
>
> if [ "$FUNC" == "uac2" ]; then
> mkdir functions/$FUNC".0"
> echo 2 > functions/$FUNC".0"/p_ssize
> echo 48000 > functions/$FUNC".0"/p_srate
> echo 3 > functions/$FUNC".0"/p_chmask
>
> echo 2 > functions/$FUNC".0"/c_ssize
> echo 48000 > functions/$FUNC".0"/c_srate
> echo 3 > functions/$FUNC".0"/c_chmask
> #echo 4 > functions/$FUNC".0"/req_number
> ln -s functions/$FUNC".0" configs/c.1
> echo high-speed > /sys/kernel/config/usb_gadget/g1/max_speed
> fi
>
Hmm... I just tested below config and it works fine with my Win10.
The only thing I noticed is Windows doesn't enable UAC2 gadget
by default, but this can be done from Win10 sound settings
--------------------------------->8--------------------------------------
mkdir cfg
mount none cfg -t configfs
mkdir cfg/usb_gadget/g1
cd cfg/usb_gadget/g1
mkdir configs/c.1
mkdir functions/uac2.0
# 44.1 kHz sample rate
echo 44100 > functions/uac2.0/c_srate
echo 44100 > functions/uac2.0/p_srate
mkdir strings/0x409
mkdir configs/c.1/strings/0x409
echo 0x0101 > idProduct
echo 0x1d6b > idVendor
echo my-serial-num > strings/0x409/serialnumber
echo my-manufacturer > strings/0x409/manufacturer
echo "Test gadget" > strings/0x409/product
echo "Conf 1" > configs/c.1/strings/0x409/configuration
echo 120 > configs/c.1/MaxPower
ln -s functions/uac2.0 configs/c.1
echo musb-hdrc.0 > UDC
--------------------------------->8--------------------------------------
Thanks,
Ruslan
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 0/3] UAC2 Gadget: feedback endpoint support
2020-11-12 23:20 ` Ruslan Bilovol
@ 2020-11-13 15:35 ` Glenn Schmottlach
2020-11-22 19:51 ` Ruslan Bilovol
0 siblings, 1 reply; 31+ messages in thread
From: Glenn Schmottlach @ 2020-11-13 15:35 UTC (permalink / raw)
To: Ruslan Bilovol; +Cc: Peter Chen, balbi, linux-usb
On Thu, Nov 12, 2020 at 6:20 PM Ruslan Bilovol <ruslan.bilovol@gmail.com> wrote:
>
> On Wed, Nov 11, 2020 at 11:30 AM Peter Chen <peter.chen@nxp.com> wrote:
> >
> > On 20-11-08 02:18:28, Ruslan Bilovol wrote:
> > > Current UAC2 gadget implements capture/sync paths
> > > as two USB ISO ASYNC endpoints (IN and OUT).
> > >
> > > This violates USB spec which says that ISO ASYNC OUT endpoint
> > > should have feedback companion endpoint.
> > > See USB2.0 spec "5.12.4.1 Synchronization Type": asynchronous
> > > sink provides explicit feedback (isochronous pipe).
> > > Interesting that for ISO ASYNC *IN* endpoint respective
> > > feedback isn't required since source provides implicit
> > > feedforward (data stream).
> > >
> > > While it's not an issue if UAC2 Gadget is connected to
> > > Linux host (Linux ignores missing feedback endpoint),
> > > with other hosts like Windows or MacOS the UAC2 Gadget
> > > isn't enumerated due to missing feedback endpoint.
> > >
> > > This patch series adds feedback endpoint support to
> > > UAC2 function, new control to UAC2 mixer which can
> > > be used by userspace tools (like alsaloop from alsa-utils)
> > > for updating feedback frequency reported to the host.
> > > This is useful for usecases when UAC2 Gadget's audio
> > > samples are played to another codec or audio card
> > > with its own internal freerunning clock so host can
> > > be notified that more/less samples are required.
> > >
> > > The alsaloop tool requires some (relatively small)
> > > modifications in order to start support driving
> > > feedback frequency through UAC2 mixer control.
> > > That change will be sent as a separate patch
> > > to ALSA community.
> > >
> > > Also added ability to switch ISO ASYNC OUT endpoint into
> > > adaptive endpoint which doesn't require feedback endpoint
> > > (as per USB spec).
> > >
> > > Ruslan Bilovol (3):
> > > usb: gadget: f_uac2/u_audio: add feedback endpoint support
> > > usb: gadget: f_uac2: add adaptive sync support for capture
> > > usb: gadget: u_audio: add real feedback implementation
> >
> > Hi Ruslan,
> >
> > I applied your patches, but WIN10 still can't recognize it well.
> > The UAC1 is OK for WIN10 with the below same configuration.
> > Any debug information you would like to know to check it?
> >
> >
> > if [ "$FUNC" == "uac2" ]; then
> > mkdir functions/$FUNC".0"
> > echo 2 > functions/$FUNC".0"/p_ssize
> > echo 48000 > functions/$FUNC".0"/p_srate
> > echo 3 > functions/$FUNC".0"/p_chmask
> >
> > echo 2 > functions/$FUNC".0"/c_ssize
> > echo 48000 > functions/$FUNC".0"/c_srate
> > echo 3 > functions/$FUNC".0"/c_chmask
> > #echo 4 > functions/$FUNC".0"/req_number
> > ln -s functions/$FUNC".0" configs/c.1
> > echo high-speed > /sys/kernel/config/usb_gadget/g1/max_speed
> > fi
> >
>
> Hmm... I just tested below config and it works fine with my Win10.
> The only thing I noticed is Windows doesn't enable UAC2 gadget
> by default, but this can be done from Win10 sound settings
>
> --------------------------------->8--------------------------------------
> mkdir cfg
> mount none cfg -t configfs
> mkdir cfg/usb_gadget/g1
> cd cfg/usb_gadget/g1
> mkdir configs/c.1
> mkdir functions/uac2.0
>
> # 44.1 kHz sample rate
> echo 44100 > functions/uac2.0/c_srate
> echo 44100 > functions/uac2.0/p_srate
>
> mkdir strings/0x409
> mkdir configs/c.1/strings/0x409
> echo 0x0101 > idProduct
> echo 0x1d6b > idVendor
> echo my-serial-num > strings/0x409/serialnumber
> echo my-manufacturer > strings/0x409/manufacturer
> echo "Test gadget" > strings/0x409/product
> echo "Conf 1" > configs/c.1/strings/0x409/configuration
> echo 120 > configs/c.1/MaxPower
> ln -s functions/uac2.0 configs/c.1
> echo musb-hdrc.0 > UDC
> --------------------------------->8--------------------------------------
>
> Thanks,
> Ruslan
Hi Ruslan -
With your configuration (above) Win10 was able to recognize and load
the driver. What appears to prevent my configuration from loading is
the fact that I selected 48K as my sample rate and my capture/playback
channel mask includes more than two (2) channels. If I take your
configuration and change the sample rate (c_srate/p_srate) to 48000
Windows will fail to load the driver. Likewise, setting the
c_chmask/p_chmask to 7 (three channels) will also cause the driver to
fail to load.
You mentioned there is an option in Win10 Sound Settings to "enable"
UAC2 by default. I cannot find that option and I wonder if this is
what is preventing me from changing the sample rate or channel mask?
Could Windows be treating my audio gadget as a UAC1 device rather than
a fully multi-channel audio device (even though the usbaudio2.sys
driver is loaded)? Have you tried other configurations to verify your
Win10 box supports more than two channels and a 44.1K sample rate? I
look forward to your feedback and any suggestions you might offer.
Thanks,
Glenn
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 0/3] UAC2 Gadget: feedback endpoint support
2020-11-13 15:35 ` Glenn Schmottlach
@ 2020-11-22 19:51 ` Ruslan Bilovol
2020-11-25 19:28 ` Glenn Schmottlach
2020-11-26 13:16 ` Jerome Brunet
0 siblings, 2 replies; 31+ messages in thread
From: Ruslan Bilovol @ 2020-11-22 19:51 UTC (permalink / raw)
To: Glenn Schmottlach; +Cc: Peter Chen, balbi, linux-usb
On Fri, Nov 13, 2020 at 5:35 PM Glenn Schmottlach
<gschmottlach@gmail.com> wrote:
>
> On Thu, Nov 12, 2020 at 6:20 PM Ruslan Bilovol <ruslan.bilovol@gmail.com> wrote:
> >
> > On Wed, Nov 11, 2020 at 11:30 AM Peter Chen <peter.chen@nxp.com> wrote:
> > >
> > > On 20-11-08 02:18:28, Ruslan Bilovol wrote:
> > > > Current UAC2 gadget implements capture/sync paths
> > > > as two USB ISO ASYNC endpoints (IN and OUT).
> > > >
> > > > This violates USB spec which says that ISO ASYNC OUT endpoint
> > > > should have feedback companion endpoint.
> > > > See USB2.0 spec "5.12.4.1 Synchronization Type": asynchronous
> > > > sink provides explicit feedback (isochronous pipe).
> > > > Interesting that for ISO ASYNC *IN* endpoint respective
> > > > feedback isn't required since source provides implicit
> > > > feedforward (data stream).
> > > >
> > > > While it's not an issue if UAC2 Gadget is connected to
> > > > Linux host (Linux ignores missing feedback endpoint),
> > > > with other hosts like Windows or MacOS the UAC2 Gadget
> > > > isn't enumerated due to missing feedback endpoint.
> > > >
> > > > This patch series adds feedback endpoint support to
> > > > UAC2 function, new control to UAC2 mixer which can
> > > > be used by userspace tools (like alsaloop from alsa-utils)
> > > > for updating feedback frequency reported to the host.
> > > > This is useful for usecases when UAC2 Gadget's audio
> > > > samples are played to another codec or audio card
> > > > with its own internal freerunning clock so host can
> > > > be notified that more/less samples are required.
> > > >
> > > > The alsaloop tool requires some (relatively small)
> > > > modifications in order to start support driving
> > > > feedback frequency through UAC2 mixer control.
> > > > That change will be sent as a separate patch
> > > > to ALSA community.
> > > >
> > > > Also added ability to switch ISO ASYNC OUT endpoint into
> > > > adaptive endpoint which doesn't require feedback endpoint
> > > > (as per USB spec).
> > > >
> > > > Ruslan Bilovol (3):
> > > > usb: gadget: f_uac2/u_audio: add feedback endpoint support
> > > > usb: gadget: f_uac2: add adaptive sync support for capture
> > > > usb: gadget: u_audio: add real feedback implementation
> > >
> > > Hi Ruslan,
> > >
> > > I applied your patches, but WIN10 still can't recognize it well.
> > > The UAC1 is OK for WIN10 with the below same configuration.
> > > Any debug information you would like to know to check it?
> > >
> > >
> > > if [ "$FUNC" == "uac2" ]; then
> > > mkdir functions/$FUNC".0"
> > > echo 2 > functions/$FUNC".0"/p_ssize
> > > echo 48000 > functions/$FUNC".0"/p_srate
> > > echo 3 > functions/$FUNC".0"/p_chmask
> > >
> > > echo 2 > functions/$FUNC".0"/c_ssize
> > > echo 48000 > functions/$FUNC".0"/c_srate
> > > echo 3 > functions/$FUNC".0"/c_chmask
> > > #echo 4 > functions/$FUNC".0"/req_number
> > > ln -s functions/$FUNC".0" configs/c.1
> > > echo high-speed > /sys/kernel/config/usb_gadget/g1/max_speed
> > > fi
> > >
> >
> > Hmm... I just tested below config and it works fine with my Win10.
> > The only thing I noticed is Windows doesn't enable UAC2 gadget
> > by default, but this can be done from Win10 sound settings
> >
> > --------------------------------->8--------------------------------------
> > mkdir cfg
> > mount none cfg -t configfs
> > mkdir cfg/usb_gadget/g1
> > cd cfg/usb_gadget/g1
> > mkdir configs/c.1
> > mkdir functions/uac2.0
> >
> > # 44.1 kHz sample rate
> > echo 44100 > functions/uac2.0/c_srate
> > echo 44100 > functions/uac2.0/p_srate
> >
> > mkdir strings/0x409
> > mkdir configs/c.1/strings/0x409
> > echo 0x0101 > idProduct
> > echo 0x1d6b > idVendor
> > echo my-serial-num > strings/0x409/serialnumber
> > echo my-manufacturer > strings/0x409/manufacturer
> > echo "Test gadget" > strings/0x409/product
> > echo "Conf 1" > configs/c.1/strings/0x409/configuration
> > echo 120 > configs/c.1/MaxPower
> > ln -s functions/uac2.0 configs/c.1
> > echo musb-hdrc.0 > UDC
> > --------------------------------->8--------------------------------------
> >
> > Thanks,
> > Ruslan
>
> Hi Ruslan -
>
> With your configuration (above) Win10 was able to recognize and load
> the driver. What appears to prevent my configuration from loading is
> the fact that I selected 48K as my sample rate and my capture/playback
> channel mask includes more than two (2) channels. If I take your
> configuration and change the sample rate (c_srate/p_srate) to 48000
> Windows will fail to load the driver. Likewise, setting the
> c_chmask/p_chmask to 7 (three channels) will also cause the driver to
> fail to load.
>
> You mentioned there is an option in Win10 Sound Settings to "enable"
> UAC2 by default. I cannot find that option and I wonder if this is
> what is preventing me from changing the sample rate or channel mask?
> Could Windows be treating my audio gadget as a UAC1 device rather than
> a fully multi-channel audio device (even though the usbaudio2.sys
> driver is loaded)? Have you tried other configurations to verify your
> Win10 box supports more than two channels and a 44.1K sample rate? I
> look forward to your feedback and any suggestions you might offer.
>
I was able to reproduce the issue and prepared a patch below, please
try it and see if it fixes the issue.
Maximum data rates that I used were (AFAIR) 8 channel 192kHz/32bit,
but there is another issue with high data rate if someone uses so many
channels, very high sampling frequency or sample size that data can't
fit into allowed (by USB spec) max packet size of endpoint. In this case
need to decrease bInterval of endpoint.
I test it in a high-performance audio case so always use a minimal possible
bInterval (set to '1').
Of course in the future that need to be calculated dynamically depending on
the UAC2 settings
Please test patches below and see it it helps
---------------------------------------------8<----------------------------------------
From 51516435bbf2486574ec7bc9fd4726677cd474a4 Mon Sep 17 00:00:00 2001
From: Ruslan Bilovol <ruslan.bilovol@gmail.com>
Date: Sun, 22 Nov 2020 21:05:38 +0200
Subject: [PATCH] usb: gadget: f_uac2: always increase endpoint max_packet_size
by one audio slot
As per UAC2 Audio Data Formats spec (2.3.1.1 USB Packets),
if the sampling rate is a constant, the allowable variation
of number of audio slots per virtual frame is +/- 1 audio slot.
It means that endpoint should be able to accept/send +1 audio
slot.
Previous endpoint max_packet_size calculation code
was adding sometimes +1 audio slot due to DIV_ROUND_UP
behaviour which was rounding up to closest integer.
However this doesn't work if the numbers are divisible.
It had no any impact with Linux hosts which ignore
this issue, but in case of more strict Windows it
caused rejected enumeration
Thus always add +1 audio slot to endpoint's max packet size
Signed-off-by: Ruslan Bilovol <ruslan.bilovol@gmail.com>
---
drivers/usb/gadget/function/f_uac2.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/usb/gadget/function/f_uac2.c
b/drivers/usb/gadget/function/f_uac2.c
index ebdb2a1..1698587 100644
--- a/drivers/usb/gadget/function/f_uac2.c
+++ b/drivers/usb/gadget/function/f_uac2.c
@@ -486,7 +486,7 @@ static void set_ep_max_packet_size(const struct
f_uac2_opts *uac2_opts,
}
max_packet_size = num_channels(chmask) * ssize *
- DIV_ROUND_UP(srate, factor / (1 << (ep_desc->bInterval - 1)));
+ ((srate / (factor / (1 << (ep_desc->bInterval - 1)))) + 1);
if (!is_playback && (uac2_opts->c_sync == USB_ENDPOINT_SYNC_ASYNC))
max_packet_size = max_packet_size * FBACK_FREQ_MAX / 100;
--
1.9.1
---------------------------------------------8<----------------------------------------
From c8f2f2b414af672ec40841e75fb1ea761ae29122 Mon Sep 17 00:00:00 2001
From: Ruslan Bilovol <ruslan.bilovol@gmail.com>
Date: Sun, 16 Feb 2020 22:40:23 +0200
Subject: [PATCH] usb: gadget: f_uac2: change bInterval to 1 for
8ch/24bit/44.1kHz
With bInterval=4 one audio slot in case of 8ch/24bit/44.1kHz
exeeds MaxPacket size of ISO endpoint (>1024) and can't be
transferred in time.
USB spec ("5.6.3 Isochronous Transfer Packet Size Constraints")
says if we need to transfer more than 1024 it is a high speed, high
bandwidth endpoint and should have bInterval=1.
In the future, bInterval should be dynamically calculated
depending on bandwith requirementes
Signed-off-by: Ruslan Bilovol <ruslan.bilovol@gmail.com>
---
drivers/usb/gadget/function/f_uac2.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/gadget/function/f_uac2.c
b/drivers/usb/gadget/function/f_uac2.c
index 3633df6..fb9b875 100644
--- a/drivers/usb/gadget/function/f_uac2.c
+++ b/drivers/usb/gadget/function/f_uac2.c
@@ -281,7 +281,7 @@ enum {
.bmAttributes = USB_ENDPOINT_XFER_ISOC | USB_ENDPOINT_SYNC_ASYNC,
.wMaxPacketSize = cpu_to_le16(1024),
- .bInterval = 4,
+ .bInterval = 1,
};
/* CS AS ISO OUT Endpoint */
@@ -358,7 +358,7 @@ enum {
.bmAttributes = USB_ENDPOINT_XFER_ISOC | USB_ENDPOINT_SYNC_ASYNC,
.wMaxPacketSize = cpu_to_le16(1024),
- .bInterval = 4,
+ .bInterval = 1,
};
/* CS AS ISO IN Endpoint */
--
1.9.1
---------------------------------------------8<----------------------------------------
Thanks,
Ruslan
^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH 0/3] UAC2 Gadget: feedback endpoint support
2020-11-22 19:51 ` Ruslan Bilovol
@ 2020-11-25 19:28 ` Glenn Schmottlach
2020-11-28 23:26 ` Ruslan Bilovol
2020-11-26 13:16 ` Jerome Brunet
1 sibling, 1 reply; 31+ messages in thread
From: Glenn Schmottlach @ 2020-11-25 19:28 UTC (permalink / raw)
To: Ruslan Bilovol; +Cc: Peter Chen, balbi, linux-usb
On Sun, Nov 22, 2020 at 2:52 PM Ruslan Bilovol <ruslan.bilovol@gmail.com> wrote:
>
> On Fri, Nov 13, 2020 at 5:35 PM Glenn Schmottlach
> <gschmottlach@gmail.com> wrote:
> >
> > On Thu, Nov 12, 2020 at 6:20 PM Ruslan Bilovol <ruslan.bilovol@gmail.com> wrote:
> > >
> > > On Wed, Nov 11, 2020 at 11:30 AM Peter Chen <peter.chen@nxp.com> wrote:
> > > >
> > > > On 20-11-08 02:18:28, Ruslan Bilovol wrote:
> > > > > Current UAC2 gadget implements capture/sync paths
> > > > > as two USB ISO ASYNC endpoints (IN and OUT).
> > > > >
> > > > > This violates USB spec which says that ISO ASYNC OUT endpoint
> > > > > should have feedback companion endpoint.
> > > > > See USB2.0 spec "5.12.4.1 Synchronization Type": asynchronous
> > > > > sink provides explicit feedback (isochronous pipe).
> > > > > Interesting that for ISO ASYNC *IN* endpoint respective
> > > > > feedback isn't required since source provides implicit
> > > > > feedforward (data stream).
> > > > >
> > > > > While it's not an issue if UAC2 Gadget is connected to
> > > > > Linux host (Linux ignores missing feedback endpoint),
> > > > > with other hosts like Windows or MacOS the UAC2 Gadget
> > > > > isn't enumerated due to missing feedback endpoint.
> > > > >
> > > > > This patch series adds feedback endpoint support to
> > > > > UAC2 function, new control to UAC2 mixer which can
> > > > > be used by userspace tools (like alsaloop from alsa-utils)
> > > > > for updating feedback frequency reported to the host.
> > > > > This is useful for usecases when UAC2 Gadget's audio
> > > > > samples are played to another codec or audio card
> > > > > with its own internal freerunning clock so host can
> > > > > be notified that more/less samples are required.
> > > > >
> > > > > The alsaloop tool requires some (relatively small)
> > > > > modifications in order to start support driving
> > > > > feedback frequency through UAC2 mixer control.
> > > > > That change will be sent as a separate patch
> > > > > to ALSA community.
> > > > >
> > > > > Also added ability to switch ISO ASYNC OUT endpoint into
> > > > > adaptive endpoint which doesn't require feedback endpoint
> > > > > (as per USB spec).
> > > > >
> > > > > Ruslan Bilovol (3):
> > > > > usb: gadget: f_uac2/u_audio: add feedback endpoint support
> > > > > usb: gadget: f_uac2: add adaptive sync support for capture
> > > > > usb: gadget: u_audio: add real feedback implementation
> > > >
> > > > Hi Ruslan,
> > > >
> > > > I applied your patches, but WIN10 still can't recognize it well.
> > > > The UAC1 is OK for WIN10 with the below same configuration.
> > > > Any debug information you would like to know to check it?
> > > >
> > > >
> > > > if [ "$FUNC" == "uac2" ]; then
> > > > mkdir functions/$FUNC".0"
> > > > echo 2 > functions/$FUNC".0"/p_ssize
> > > > echo 48000 > functions/$FUNC".0"/p_srate
> > > > echo 3 > functions/$FUNC".0"/p_chmask
> > > >
> > > > echo 2 > functions/$FUNC".0"/c_ssize
> > > > echo 48000 > functions/$FUNC".0"/c_srate
> > > > echo 3 > functions/$FUNC".0"/c_chmask
> > > > #echo 4 > functions/$FUNC".0"/req_number
> > > > ln -s functions/$FUNC".0" configs/c.1
> > > > echo high-speed > /sys/kernel/config/usb_gadget/g1/max_speed
> > > > fi
> > > >
> > >
> > > Hmm... I just tested below config and it works fine with my Win10.
> > > The only thing I noticed is Windows doesn't enable UAC2 gadget
> > > by default, but this can be done from Win10 sound settings
> > >
> > > --------------------------------->8--------------------------------------
> > > mkdir cfg
> > > mount none cfg -t configfs
> > > mkdir cfg/usb_gadget/g1
> > > cd cfg/usb_gadget/g1
> > > mkdir configs/c.1
> > > mkdir functions/uac2.0
> > >
> > > # 44.1 kHz sample rate
> > > echo 44100 > functions/uac2.0/c_srate
> > > echo 44100 > functions/uac2.0/p_srate
> > >
> > > mkdir strings/0x409
> > > mkdir configs/c.1/strings/0x409
> > > echo 0x0101 > idProduct
> > > echo 0x1d6b > idVendor
> > > echo my-serial-num > strings/0x409/serialnumber
> > > echo my-manufacturer > strings/0x409/manufacturer
> > > echo "Test gadget" > strings/0x409/product
> > > echo "Conf 1" > configs/c.1/strings/0x409/configuration
> > > echo 120 > configs/c.1/MaxPower
> > > ln -s functions/uac2.0 configs/c.1
> > > echo musb-hdrc.0 > UDC
> > > --------------------------------->8--------------------------------------
> > >
> > > Thanks,
> > > Ruslan
> >
> > Hi Ruslan -
> >
> > With your configuration (above) Win10 was able to recognize and load
> > the driver. What appears to prevent my configuration from loading is
> > the fact that I selected 48K as my sample rate and my capture/playback
> > channel mask includes more than two (2) channels. If I take your
> > configuration and change the sample rate (c_srate/p_srate) to 48000
> > Windows will fail to load the driver. Likewise, setting the
> > c_chmask/p_chmask to 7 (three channels) will also cause the driver to
> > fail to load.
> >
> > You mentioned there is an option in Win10 Sound Settings to "enable"
> > UAC2 by default. I cannot find that option and I wonder if this is
> > what is preventing me from changing the sample rate or channel mask?
> > Could Windows be treating my audio gadget as a UAC1 device rather than
> > a fully multi-channel audio device (even though the usbaudio2.sys
> > driver is loaded)? Have you tried other configurations to verify your
> > Win10 box supports more than two channels and a 44.1K sample rate? I
> > look forward to your feedback and any suggestions you might offer.
> >
>
> I was able to reproduce the issue and prepared a patch below, please
> try it and see if it fixes the issue.
>
> Maximum data rates that I used were (AFAIR) 8 channel 192kHz/32bit,
> but there is another issue with high data rate if someone uses so many
> channels, very high sampling frequency or sample size that data can't
> fit into allowed (by USB spec) max packet size of endpoint. In this case
> need to decrease bInterval of endpoint.
> I test it in a high-performance audio case so always use a minimal possible
> bInterval (set to '1').
> Of course in the future that need to be calculated dynamically depending on
> the UAC2 settings
>
> Please test patches below and see it it helps
>
> ---------------------------------------------8<----------------------------------------
> From 51516435bbf2486574ec7bc9fd4726677cd474a4 Mon Sep 17 00:00:00 2001
> From: Ruslan Bilovol <ruslan.bilovol@gmail.com>
> Date: Sun, 22 Nov 2020 21:05:38 +0200
> Subject: [PATCH] usb: gadget: f_uac2: always increase endpoint max_packet_size
> by one audio slot
>
> As per UAC2 Audio Data Formats spec (2.3.1.1 USB Packets),
> if the sampling rate is a constant, the allowable variation
> of number of audio slots per virtual frame is +/- 1 audio slot.
>
> It means that endpoint should be able to accept/send +1 audio
> slot.
>
> Previous endpoint max_packet_size calculation code
> was adding sometimes +1 audio slot due to DIV_ROUND_UP
> behaviour which was rounding up to closest integer.
> However this doesn't work if the numbers are divisible.
>
> It had no any impact with Linux hosts which ignore
> this issue, but in case of more strict Windows it
> caused rejected enumeration
>
> Thus always add +1 audio slot to endpoint's max packet size
>
> Signed-off-by: Ruslan Bilovol <ruslan.bilovol@gmail.com>
> ---
> drivers/usb/gadget/function/f_uac2.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/gadget/function/f_uac2.c
> b/drivers/usb/gadget/function/f_uac2.c
> index ebdb2a1..1698587 100644
> --- a/drivers/usb/gadget/function/f_uac2.c
> +++ b/drivers/usb/gadget/function/f_uac2.c
> @@ -486,7 +486,7 @@ static void set_ep_max_packet_size(const struct
> f_uac2_opts *uac2_opts,
> }
>
> max_packet_size = num_channels(chmask) * ssize *
> - DIV_ROUND_UP(srate, factor / (1 << (ep_desc->bInterval - 1)));
> + ((srate / (factor / (1 << (ep_desc->bInterval - 1)))) + 1);
>
> if (!is_playback && (uac2_opts->c_sync == USB_ENDPOINT_SYNC_ASYNC))
> max_packet_size = max_packet_size * FBACK_FREQ_MAX / 100;
> --
> 1.9.1
>
>
> ---------------------------------------------8<----------------------------------------
> From c8f2f2b414af672ec40841e75fb1ea761ae29122 Mon Sep 17 00:00:00 2001
> From: Ruslan Bilovol <ruslan.bilovol@gmail.com>
> Date: Sun, 16 Feb 2020 22:40:23 +0200
> Subject: [PATCH] usb: gadget: f_uac2: change bInterval to 1 for
> 8ch/24bit/44.1kHz
>
> With bInterval=4 one audio slot in case of 8ch/24bit/44.1kHz
> exeeds MaxPacket size of ISO endpoint (>1024) and can't be
> transferred in time.
> USB spec ("5.6.3 Isochronous Transfer Packet Size Constraints")
> says if we need to transfer more than 1024 it is a high speed, high
> bandwidth endpoint and should have bInterval=1.
>
> In the future, bInterval should be dynamically calculated
> depending on bandwith requirementes
>
> Signed-off-by: Ruslan Bilovol <ruslan.bilovol@gmail.com>
> ---
> drivers/usb/gadget/function/f_uac2.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/gadget/function/f_uac2.c
> b/drivers/usb/gadget/function/f_uac2.c
> index 3633df6..fb9b875 100644
> --- a/drivers/usb/gadget/function/f_uac2.c
> +++ b/drivers/usb/gadget/function/f_uac2.c
> @@ -281,7 +281,7 @@ enum {
>
> .bmAttributes = USB_ENDPOINT_XFER_ISOC | USB_ENDPOINT_SYNC_ASYNC,
> .wMaxPacketSize = cpu_to_le16(1024),
> - .bInterval = 4,
> + .bInterval = 1,
> };
>
> /* CS AS ISO OUT Endpoint */
> @@ -358,7 +358,7 @@ enum {
>
> .bmAttributes = USB_ENDPOINT_XFER_ISOC | USB_ENDPOINT_SYNC_ASYNC,
> .wMaxPacketSize = cpu_to_le16(1024),
> - .bInterval = 4,
> + .bInterval = 1,
> };
>
> /* CS AS ISO IN Endpoint */
> --
> 1.9.1
>
>
> ---------------------------------------------8<----------------------------------------
>
> Thanks,
> Ruslan
Hi Ruslan -
I have had some mixed success with your additional patches. First, the
UAC2 gadget does correctly register with Windows 10 but only for 1-31
channels. For a fully specified channel map (32 channels):
c_chmask = 0xFFFFFFFF ( Does not load)
p_chmask = 0xFFFFFFFF ( Does not load)
the driver fails to load (code=10). This seems similar to the prior
problem. Could there be another off-by-one problem here?
With a 31 channel mask the Windows 10 usbaudio2.sys loads successfully, e.g.
c_chmask = 0x7FFFFFFF (loads)
p_chmask = 0x7FFFFFFF (loads)
Once Windows 10 loads the driver successfully, however, I can't seem
to select the device through Audacity on Windows. The device appears
in the Windows "Device Manager" under "Sound, video and game
controllers -> Source/Sink". How are you testing audio
playback/capture under Windows? Perhaps my test approach is wrong?
I used the same approach on my Linux host where the USB UAC2 device
appears as an ALSA device and I use speaker-test on the target
(BeagleBone Black) to play and then capture the audio using Audacity
on the Linux host. I was hoping to do the same or something similar
under Windows.
Below is the script I've been using on the target to configure my UAC2 device:
mkdir -p "/sys/kernel/config/usb_gadget/g_multi"
echo "${idVendor:-0x11F5}" > "/sys/kernel/config/usb_gadget/g_multi/idVendor"
echo "${idProduct:-0x0009}" > "/sys/kernel/config/usb_gadget/g_multi/idProduct"
echo "${bcdDevice:-0x0001}" > "/sys/kernel/config/usb_gadget/g_multi/bcdDevice"
echo "${bDeviceClass:-0x00}" >
"/sys/kernel/config/usb_gadget/g_multi/bDeviceClass"
echo "${bDeviceSubClass:-0x00}" >
"/sys/kernel/config/usb_gadget/g_multi/bDeviceSubClass"
echo "${bDeviceProtocol:-0x00}" >
"/sys/kernel/config/usb_gadget/g_multi/bDeviceProtocol"
echo "${bcdUSB:-0x0200}" > "/sys/kernel/config/usb_gadget/g_multi/bcdUSB"
echo "${bMaxPacketSize0:-0x00}" >
"/sys/kernel/config/usb_gadget/g_multi/bMaxPacketSize0"
mkdir -p "/sys/kernel/config/usb_gadget/g_multi/strings/0x409"
echo "${serialnumber:-000000000000}" >
"/sys/kernel/config/usb_gadget/g_multi/strings/0x409/serialnumber"
echo "${product:-UAC2}" >
"/sys/kernel/config/usb_gadget/g_multi/strings/0x409/product"
echo "${manufacturer:-www.acme.com}" >
"/sys/kernel/config/usb_gadget/g_multi/strings/0x409/manufacturer"
mkdir -p "/sys/kernel/config/usb_gadget/g_multi/functions/uac2.usb0"
echo "${req_number:-4}" >
"/sys/kernel/config/usb_gadget/g_multi/functions/uac2.usb0/req_number"
echo "${c_ssize:-4}" >
"/sys/kernel/config/usb_gadget/g_multi/functions/uac2.usb0/c_ssize"
echo "${c_srate:-48000}" >
"/sys/kernel/config/usb_gadget/g_multi/functions/uac2.usb0/c_srate"
echo "${c_chmask:-0x00000003}" >
"/sys/kernel/config/usb_gadget/g_multi/functions/uac2.usb0/c_chmask"
echo "${c_sync:-asynchronous}" >
"/sys/kernel/config/usb_gadget/g_multi/functions/uac2.usb0/c_sync"
echo "${p_ssize:-4}" >
"/sys/kernel/config/usb_gadget/g_multi/functions/uac2.usb0/p_ssize"
echo "${p_srate:-48000}" >
"/sys/kernel/config/usb_gadget/g_multi/functions/uac2.usb0/p_srate"
echo "${p_chmask:-0x00000003}" >
"/sys/kernel/config/usb_gadget/g_multi/functions/uac2.usb0/p_chmask"
mkdir -p "/sys/kernel/config/usb_gadget/g_multi/configs/c.1"
echo "${bmAttributes:-0x80}" >
"/sys/kernel/config/usb_gadget/g_multi/configs/c.1/bmAttributes"
echo "${MaxPower:-500}" >
"/sys/kernel/config/usb_gadget/g_multi/configs/c.1/MaxPower"
mkdir -p "/sys/kernel/config/usb_gadget/g_multi/configs/c.1/strings/0x409"
echo "${configuration:-BeagleBone Composite}" >
"/sys/kernel/config/usb_gadget/g_multi/configs/c.1/strings/0x409/configuration"
ln -s "/sys/kernel/config/usb_gadget/g_multi/functions/uac2.usb0"
"/sys/kernel/config/usb_gadget/g_multi/configs/c.1/uac2.usb0"
mkdir -p "/sys/kernel/config/usb_gadget/g_multi/os_desc"
ln -s "//sys/kernel/config/usb_gadget/g_multi/configs/c.1"
"/sys/kernel/config/usb_gadget/g_multi/os_desc/c.1"
echo "${qw_sign:-MSFT100}" >
"/sys/kernel/config/usb_gadget/g_multi/os_desc/qw_sign"
echo "${b_vendor_code:-0x00}" >
"/sys/kernel/config/usb_gadget/g_multi/os_desc/b_vendor_code"
echo "${use:-1}" > "/sys/kernel/config/usb_gadget/g_multi/os_desc/use"
basename /sys/class/udc/* > /sys/kernel/config/usb_gadget/g_multi/UDC
This is only a two channel playback/capture configuration. I hoped to
get this working under Windows before increasing my channel count to a
full 32 channel mask with a 48K sample rate.
I look forward to hearing your suggestions and possibly pointing out
where I've gone astray.
Thanks,
Glenn
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 2/3] usb: gadget: f_uac2: add adaptive sync support for capture
2020-11-08 0:18 ` [PATCH 2/3] usb: gadget: f_uac2: add adaptive sync support for capture Ruslan Bilovol
2020-11-11 9:18 ` Peter Chen
@ 2020-11-26 11:13 ` Jerome Brunet
2020-12-04 14:03 ` Ruslan Bilovol
1 sibling, 1 reply; 31+ messages in thread
From: Jerome Brunet @ 2020-11-26 11:13 UTC (permalink / raw)
To: Ruslan Bilovol, balbi; +Cc: linux-usb, gschmottlach
On Sun 08 Nov 2020 at 01:18, Ruslan Bilovol <ruslan.bilovol@gmail.com> wrote:
> Current f_uac2 USB OUT (aka 'capture') synchronization
> implements 'ASYNC' scenario which means USB Gadget has
> it's own freerunning clock and can update Host about
> real clock frequency through feedback endpoint so Host
> can align number of samples sent to the USB gadget to
> prevent overruns/underruns
>
> In case if Gadget can has no it's internal clock and
> can consume audio samples at any rate (for example,
> on the Gadget side someone records audio directly to
> a file, or audio samples are played through an
> external DAC as soon as they arrive), UAC2 spec
> suggests 'ADAPTIVE' synchronization type.
>
> Change UAC2 driver to make it configurable through
> additional 'c_sync' configfs file.
>
> Default remains 'asynchronous' with possibility to
> switch it to 'adaptive'
>
> Signed-off-by: Ruslan Bilovol <ruslan.bilovol@gmail.com>
> ---
> Documentation/ABI/testing/configfs-usb-gadget-uac2 | 1 +
> Documentation/usb/gadget-testing.rst | 1 +
> drivers/usb/gadget/function/f_uac2.c | 96 ++++++++++++++++++++--
> drivers/usb/gadget/function/u_uac2.h | 2 +
> 4 files changed, 91 insertions(+), 9 deletions(-)
>
> diff --git a/Documentation/ABI/testing/configfs-usb-gadget-uac2 b/Documentation/ABI/testing/configfs-usb-gadget-uac2
> index 2bfdd4e..4fbff96 100644
> --- a/Documentation/ABI/testing/configfs-usb-gadget-uac2
> +++ b/Documentation/ABI/testing/configfs-usb-gadget-uac2
> @@ -7,6 +7,7 @@ Description:
> c_chmask - capture channel mask
> c_srate - capture sampling rate
> c_ssize - capture sample size (bytes)
> + c_sync - capture synchronization type (async/adaptive)
> p_chmask - playback channel mask
> p_srate - playback sampling rate
> p_ssize - playback sample size (bytes)
> diff --git a/Documentation/usb/gadget-testing.rst b/Documentation/usb/gadget-testing.rst
> index 2eeb3e9..360a7ca 100644
> --- a/Documentation/usb/gadget-testing.rst
> +++ b/Documentation/usb/gadget-testing.rst
> @@ -728,6 +728,7 @@ The uac2 function provides these attributes in its function directory:
> c_chmask capture channel mask
> c_srate capture sampling rate
> c_ssize capture sample size (bytes)
> + c_sync capture synchronization type (async/adaptive)
> p_chmask playback channel mask
> p_srate playback sampling rate
> p_ssize playback sample size (bytes)
> diff --git a/drivers/usb/gadget/function/f_uac2.c b/drivers/usb/gadget/function/f_uac2.c
> index a57bf77..3187ad3 100644
> --- a/drivers/usb/gadget/function/f_uac2.c
> +++ b/drivers/usb/gadget/function/f_uac2.c
> @@ -41,6 +41,7 @@
>
> #define EPIN_EN(_opts) ((_opts)->p_chmask != 0)
> #define EPOUT_EN(_opts) ((_opts)->c_chmask != 0)
> +#define EPOUT_FBACK_IN_EN(_opts) ((_opts)->c_sync == USB_ENDPOINT_SYNC_ASYNC)
>
> struct f_uac2 {
> struct g_audio g_audio;
> @@ -237,7 +238,7 @@ enum {
> .bDescriptorType = USB_DT_INTERFACE,
>
> .bAlternateSetting = 1,
> - .bNumEndpoints = 2,
> + .bNumEndpoints = 1,
> .bInterfaceClass = USB_CLASS_AUDIO,
> .bInterfaceSubClass = USB_SUBCLASS_AUDIOSTREAMING,
> .bInterfaceProtocol = UAC_VERSION_2,
> @@ -270,7 +271,7 @@ enum {
> .bDescriptorType = USB_DT_ENDPOINT,
>
> .bEndpointAddress = USB_DIR_OUT,
> - .bmAttributes = USB_ENDPOINT_XFER_ISOC | USB_ENDPOINT_SYNC_ASYNC,
> + .bmAttributes = USB_ENDPOINT_XFER_ISOC,
> .wMaxPacketSize = cpu_to_le16(1023),
> .bInterval = 1,
> };
> @@ -279,7 +280,7 @@ enum {
> .bLength = USB_DT_ENDPOINT_SIZE,
> .bDescriptorType = USB_DT_ENDPOINT,
>
> - .bmAttributes = USB_ENDPOINT_XFER_ISOC | USB_ENDPOINT_SYNC_ASYNC,
> + .bmAttributes = USB_ENDPOINT_XFER_ISOC,
> .wMaxPacketSize = cpu_to_le16(1024),
> .bInterval = 1,
> };
> @@ -540,6 +541,19 @@ static void setup_descriptor(struct f_uac2_opts *opts)
> len += sizeof(io_out_ot_desc);
> ac_hdr_desc.wTotalLength = cpu_to_le16(len);
> iad_desc.bInterfaceCount++;
> +
> + if (EPOUT_FBACK_IN_EN(opts)) {
> + fs_epout_desc.bmAttributes |=
> + USB_ENDPOINT_SYNC_ASYNC;
> + hs_epout_desc.bmAttributes |=
> + USB_ENDPOINT_SYNC_ASYNC;
> + std_as_out_if1_desc.bNumEndpoints++;
> + } else {
> + fs_epout_desc.bmAttributes |=
> + USB_ENDPOINT_SYNC_ADAPTIVE;
> + hs_epout_desc.bmAttributes |=
> + USB_ENDPOINT_SYNC_ADAPTIVE;
> + }
Hi Ruslan,
First, thanks a lot for this series, it is very helpful
Instead of amending the descriptors, could you consider using comments like
/* .bNumEndpoints = DYNAMIC */
or
/* .bmAttributes = DYNAMIC */
It would help understand that the actual value of these parameters will be
determine at runtime, making the code easier to follow and maintain.
Also, I wonder if it would be difficult to add implicit feedback support
while at it ?
I'm asking this now because it could (possibly) change the driver params
(implicit is async as well)
> }
>
> i = 0;
> @@ -564,7 +578,8 @@ static void setup_descriptor(struct f_uac2_opts *opts)
> fs_audio_desc[i++] = USBDHDR(&as_out_fmt1_desc);
> fs_audio_desc[i++] = USBDHDR(&fs_epout_desc);
> fs_audio_desc[i++] = USBDHDR(&as_iso_out_desc);
> - fs_audio_desc[i++] = USBDHDR(&fs_epin_fback_desc);
> + if (EPOUT_FBACK_IN_EN(opts))
> + fs_audio_desc[i++] = USBDHDR(&fs_epin_fback_desc);
> }
> if (EPIN_EN(opts)) {
> fs_audio_desc[i++] = USBDHDR(&std_as_in_if0_desc);
> @@ -598,7 +613,8 @@ static void setup_descriptor(struct f_uac2_opts *opts)
> hs_audio_desc[i++] = USBDHDR(&as_out_fmt1_desc);
> hs_audio_desc[i++] = USBDHDR(&hs_epout_desc);
> hs_audio_desc[i++] = USBDHDR(&as_iso_out_desc);
> - hs_audio_desc[i++] = USBDHDR(&hs_epin_fback_desc);
> + if (EPOUT_FBACK_IN_EN(opts))
> + hs_audio_desc[i++] = USBDHDR(&hs_epin_fback_desc);
> }
> if (EPIN_EN(opts)) {
> hs_audio_desc[i++] = USBDHDR(&std_as_in_if0_desc);
> @@ -706,11 +722,14 @@ static void setup_descriptor(struct f_uac2_opts *opts)
> dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
> return -ENODEV;
> }
> - agdev->in_ep_fback = usb_ep_autoconfig(gadget,
> + if (EPOUT_FBACK_IN_EN(uac2_opts)) {
> + agdev->in_ep_fback = usb_ep_autoconfig(gadget,
> &fs_epin_fback_desc);
> - if (!agdev->in_ep_fback) {
> - dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
> - return -ENODEV;
> + if (!agdev->in_ep_fback) {
> + dev_err(dev, "%s:%d Error!\n",
> + __func__, __LINE__);
> + return -ENODEV;
> + }
> }
> }
>
> @@ -1057,11 +1076,68 @@ static void f_uac2_attr_release(struct config_item *item)
> \
> CONFIGFS_ATTR(f_uac2_opts_, name)
>
> +#define UAC2_ATTRIBUTE_SYNC(name) \
> +static ssize_t f_uac2_opts_##name##_show(struct config_item *item, \
> + char *page) \
> +{ \
> + struct f_uac2_opts *opts = to_f_uac2_opts(item); \
> + int result; \
> + char *str; \
> + \
> + mutex_lock(&opts->lock); \
> + switch (opts->name) { \
> + case USB_ENDPOINT_SYNC_ASYNC: \
> + str = "async"; \
> + break; \
> + case USB_ENDPOINT_SYNC_ADAPTIVE: \
> + str = "adaptive"; \
> + break; \
> + default: \
> + str = "unknown"; \
> + break; \
> + } \
> + result = sprintf(page, "%s\n", str); \
> + mutex_unlock(&opts->lock); \
> + \
> + return result; \
> +} \
> + \
> +static ssize_t f_uac2_opts_##name##_store(struct config_item *item, \
> + const char *page, size_t len) \
> +{ \
> + struct f_uac2_opts *opts = to_f_uac2_opts(item); \
> + int ret = 0; \
> + \
> + mutex_lock(&opts->lock); \
> + if (opts->refcnt) { \
> + ret = -EBUSY; \
> + goto end; \
> + } \
> + \
> + if (!strncmp(page, "async", 5)) \
> + opts->name = USB_ENDPOINT_SYNC_ASYNC; \
> + else if (!strncmp(page, "adaptive", 8)) \
> + opts->name = USB_ENDPOINT_SYNC_ADAPTIVE; \
> + else { \
> + ret = -EINVAL; \
> + goto end; \
> + } \
> + \
> + ret = len; \
> + \
> +end: \
> + mutex_unlock(&opts->lock); \
> + return ret; \
> +} \
> + \
> +CONFIGFS_ATTR(f_uac2_opts_, name)
> +
> UAC2_ATTRIBUTE(p_chmask);
> UAC2_ATTRIBUTE(p_srate);
> UAC2_ATTRIBUTE(p_ssize);
> UAC2_ATTRIBUTE(c_chmask);
> UAC2_ATTRIBUTE(c_srate);
> +UAC2_ATTRIBUTE_SYNC(c_sync);
> UAC2_ATTRIBUTE(c_ssize);
> UAC2_ATTRIBUTE(req_number);
>
> @@ -1072,6 +1148,7 @@ static void f_uac2_attr_release(struct config_item *item)
> &f_uac2_opts_attr_c_chmask,
> &f_uac2_opts_attr_c_srate,
> &f_uac2_opts_attr_c_ssize,
> + &f_uac2_opts_attr_c_sync,
> &f_uac2_opts_attr_req_number,
> NULL,
> };
> @@ -1110,6 +1187,7 @@ static struct usb_function_instance *afunc_alloc_inst(void)
> opts->c_chmask = UAC2_DEF_CCHMASK;
> opts->c_srate = UAC2_DEF_CSRATE;
> opts->c_ssize = UAC2_DEF_CSSIZE;
> + opts->c_sync = UAC2_DEF_CSYNC;
> opts->req_number = UAC2_DEF_REQ_NUM;
> return &opts->func_inst;
> }
> diff --git a/drivers/usb/gadget/function/u_uac2.h b/drivers/usb/gadget/function/u_uac2.h
> index b503571..13589c3 100644
> --- a/drivers/usb/gadget/function/u_uac2.h
> +++ b/drivers/usb/gadget/function/u_uac2.h
> @@ -21,6 +21,7 @@
> #define UAC2_DEF_CCHMASK 0x3
> #define UAC2_DEF_CSRATE 64000
> #define UAC2_DEF_CSSIZE 2
> +#define UAC2_DEF_CSYNC USB_ENDPOINT_SYNC_ASYNC
> #define UAC2_DEF_REQ_NUM 2
>
> struct f_uac2_opts {
> @@ -31,6 +32,7 @@ struct f_uac2_opts {
> int c_chmask;
> int c_srate;
> int c_ssize;
> + int c_sync;
> int req_number;
> bool bound;
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 0/3] UAC2 Gadget: feedback endpoint support
2020-11-22 19:51 ` Ruslan Bilovol
2020-11-25 19:28 ` Glenn Schmottlach
@ 2020-11-26 13:16 ` Jerome Brunet
2020-11-26 13:44 ` Pavel Hofman
2020-12-04 14:35 ` Ruslan Bilovol
1 sibling, 2 replies; 31+ messages in thread
From: Jerome Brunet @ 2020-11-26 13:16 UTC (permalink / raw)
To: Ruslan Bilovol, Glenn Schmottlach; +Cc: Peter Chen, balbi, linux-usb
On Sun 22 Nov 2020 at 20:51, Ruslan Bilovol <ruslan.bilovol@gmail.com> wrote:
> On Fri, Nov 13, 2020 at 5:35 PM Glenn Schmottlach
> <gschmottlach@gmail.com> wrote:
>>
>> On Thu, Nov 12, 2020 at 6:20 PM Ruslan Bilovol <ruslan.bilovol@gmail.com> wrote:
>> >
>> > On Wed, Nov 11, 2020 at 11:30 AM Peter Chen <peter.chen@nxp.com> wrote:
>> > >
>> > > On 20-11-08 02:18:28, Ruslan Bilovol wrote:
>> > > > Current UAC2 gadget implements capture/sync paths
>> > > > as two USB ISO ASYNC endpoints (IN and OUT).
>> > > >
>> > > > This violates USB spec which says that ISO ASYNC OUT endpoint
>> > > > should have feedback companion endpoint.
>> > > > See USB2.0 spec "5.12.4.1 Synchronization Type": asynchronous
>> > > > sink provides explicit feedback (isochronous pipe).
>> > > > Interesting that for ISO ASYNC *IN* endpoint respective
>> > > > feedback isn't required since source provides implicit
>> > > > feedforward (data stream).
>> > > >
>> > > > While it's not an issue if UAC2 Gadget is connected to
>> > > > Linux host (Linux ignores missing feedback endpoint),
>> > > > with other hosts like Windows or MacOS the UAC2 Gadget
>> > > > isn't enumerated due to missing feedback endpoint.
>> > > >
>> > > > This patch series adds feedback endpoint support to
>> > > > UAC2 function, new control to UAC2 mixer which can
>> > > > be used by userspace tools (like alsaloop from alsa-utils)
>> > > > for updating feedback frequency reported to the host.
>> > > > This is useful for usecases when UAC2 Gadget's audio
>> > > > samples are played to another codec or audio card
>> > > > with its own internal freerunning clock so host can
>> > > > be notified that more/less samples are required.
>> > > >
>> > > > The alsaloop tool requires some (relatively small)
>> > > > modifications in order to start support driving
>> > > > feedback frequency through UAC2 mixer control.
>> > > > That change will be sent as a separate patch
>> > > > to ALSA community.
>> > > >
>> > > > Also added ability to switch ISO ASYNC OUT endpoint into
>> > > > adaptive endpoint which doesn't require feedback endpoint
>> > > > (as per USB spec).
>> > > >
>> > > > Ruslan Bilovol (3):
>> > > > usb: gadget: f_uac2/u_audio: add feedback endpoint support
>> > > > usb: gadget: f_uac2: add adaptive sync support for capture
>> > > > usb: gadget: u_audio: add real feedback implementation
>> > >
>> > > Hi Ruslan,
>> > >
>> > > I applied your patches, but WIN10 still can't recognize it well.
>> > > The UAC1 is OK for WIN10 with the below same configuration.
>> > > Any debug information you would like to know to check it?
>> > >
>> > >
>> > > if [ "$FUNC" == "uac2" ]; then
>> > > mkdir functions/$FUNC".0"
>> > > echo 2 > functions/$FUNC".0"/p_ssize
>> > > echo 48000 > functions/$FUNC".0"/p_srate
>> > > echo 3 > functions/$FUNC".0"/p_chmask
>> > >
>> > > echo 2 > functions/$FUNC".0"/c_ssize
>> > > echo 48000 > functions/$FUNC".0"/c_srate
>> > > echo 3 > functions/$FUNC".0"/c_chmask
>> > > #echo 4 > functions/$FUNC".0"/req_number
>> > > ln -s functions/$FUNC".0" configs/c.1
>> > > echo high-speed > /sys/kernel/config/usb_gadget/g1/max_speed
>> > > fi
>> > >
>> >
>> > Hmm... I just tested below config and it works fine with my Win10.
>> > The only thing I noticed is Windows doesn't enable UAC2 gadget
>> > by default, but this can be done from Win10 sound settings
>> >
>> > --------------------------------->8--------------------------------------
>> > mkdir cfg
>> > mount none cfg -t configfs
>> > mkdir cfg/usb_gadget/g1
>> > cd cfg/usb_gadget/g1
>> > mkdir configs/c.1
>> > mkdir functions/uac2.0
>> >
>> > # 44.1 kHz sample rate
>> > echo 44100 > functions/uac2.0/c_srate
>> > echo 44100 > functions/uac2.0/p_srate
>> >
>> > mkdir strings/0x409
>> > mkdir configs/c.1/strings/0x409
>> > echo 0x0101 > idProduct
>> > echo 0x1d6b > idVendor
>> > echo my-serial-num > strings/0x409/serialnumber
>> > echo my-manufacturer > strings/0x409/manufacturer
>> > echo "Test gadget" > strings/0x409/product
>> > echo "Conf 1" > configs/c.1/strings/0x409/configuration
>> > echo 120 > configs/c.1/MaxPower
>> > ln -s functions/uac2.0 configs/c.1
>> > echo musb-hdrc.0 > UDC
>> > --------------------------------->8--------------------------------------
>> >
>> > Thanks,
>> > Ruslan
>>
>> Hi Ruslan -
>>
>> With your configuration (above) Win10 was able to recognize and load
>> the driver. What appears to prevent my configuration from loading is
>> the fact that I selected 48K as my sample rate and my capture/playback
>> channel mask includes more than two (2) channels. If I take your
>> configuration and change the sample rate (c_srate/p_srate) to 48000
>> Windows will fail to load the driver. Likewise, setting the
>> c_chmask/p_chmask to 7 (three channels) will also cause the driver to
>> fail to load.
>>
>> You mentioned there is an option in Win10 Sound Settings to "enable"
>> UAC2 by default. I cannot find that option and I wonder if this is
>> what is preventing me from changing the sample rate or channel mask?
>> Could Windows be treating my audio gadget as a UAC1 device rather than
>> a fully multi-channel audio device (even though the usbaudio2.sys
>> driver is loaded)? Have you tried other configurations to verify your
>> Win10 box supports more than two channels and a 44.1K sample rate? I
>> look forward to your feedback and any suggestions you might offer.
>>
>
> I was able to reproduce the issue and prepared a patch below, please
> try it and see if it fixes the issue.
>
> Maximum data rates that I used were (AFAIR) 8 channel 192kHz/32bit,
> but there is another issue with high data rate if someone uses so many
> channels, very high sampling frequency or sample size that data can't
> fit into allowed (by USB spec) max packet size of endpoint. In this case
> need to decrease bInterval of endpoint.
> I test it in a high-performance audio case so always use a minimal possible
> bInterval (set to '1').
> Of course in the future that need to be calculated dynamically depending on
> the UAC2 settings
>
> Please test patches below and see it it helps
>
> ---------------------------------------------8<----------------------------------------
> From 51516435bbf2486574ec7bc9fd4726677cd474a4 Mon Sep 17 00:00:00 2001
> From: Ruslan Bilovol <ruslan.bilovol@gmail.com>
> Date: Sun, 22 Nov 2020 21:05:38 +0200
> Subject: [PATCH] usb: gadget: f_uac2: always increase endpoint max_packet_size
> by one audio slot
>
> As per UAC2 Audio Data Formats spec (2.3.1.1 USB Packets),
> if the sampling rate is a constant, the allowable variation
> of number of audio slots per virtual frame is +/- 1 audio slot.
>
> It means that endpoint should be able to accept/send +1 audio
> slot.
>
> Previous endpoint max_packet_size calculation code
> was adding sometimes +1 audio slot due to DIV_ROUND_UP
> behaviour which was rounding up to closest integer.
> However this doesn't work if the numbers are divisible.
>
> It had no any impact with Linux hosts which ignore
> this issue, but in case of more strict Windows it
> caused rejected enumeration
>
> Thus always add +1 audio slot to endpoint's max packet size
>
> Signed-off-by: Ruslan Bilovol <ruslan.bilovol@gmail.com>
> ---
> drivers/usb/gadget/function/f_uac2.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/usb/gadget/function/f_uac2.c
> b/drivers/usb/gadget/function/f_uac2.c
> index ebdb2a1..1698587 100644
> --- a/drivers/usb/gadget/function/f_uac2.c
> +++ b/drivers/usb/gadget/function/f_uac2.c
> @@ -486,7 +486,7 @@ static void set_ep_max_packet_size(const struct
> f_uac2_opts *uac2_opts,
> }
>
> max_packet_size = num_channels(chmask) * ssize *
> - DIV_ROUND_UP(srate, factor / (1 << (ep_desc->bInterval - 1)));
> + ((srate / (factor / (1 << (ep_desc->bInterval - 1)))) + 1);
>
> if (!is_playback && (uac2_opts->c_sync == USB_ENDPOINT_SYNC_ASYNC))
> max_packet_size = max_packet_size * FBACK_FREQ_MAX / 100;
I think "reserving" 20% additional bandwidth is a bit extreme.
In the end, the purpose is composate the drift which appears between
different XTALs
Allocating bandwidth for 1 more sample than the stream should require
(49 instead of 48 for 48kHz with 4 bIntervals) should allow to
compensate any realistic drift, don't you think ?
> --
> 1.9.1
>
>
> ---------------------------------------------8<----------------------------------------
> From c8f2f2b414af672ec40841e75fb1ea761ae29122 Mon Sep 17 00:00:00 2001
> From: Ruslan Bilovol <ruslan.bilovol@gmail.com>
> Date: Sun, 16 Feb 2020 22:40:23 +0200
> Subject: [PATCH] usb: gadget: f_uac2: change bInterval to 1 for
> 8ch/24bit/44.1kHz
>
> With bInterval=4 one audio slot in case of 8ch/24bit/44.1kHz
> exeeds MaxPacket size of ISO endpoint (>1024) and can't be
> transferred in time.
> USB spec ("5.6.3 Isochronous Transfer Packet Size Constraints")
> says if we need to transfer more than 1024 it is a high speed, high
> bandwidth endpoint and should have bInterval=1.
>
> In the future, bInterval should be dynamically calculated
> depending on bandwith requirementes
Probably better to do it from the start to avoid breaking stuff for
people who have been using the gadget with the current badnwidth so far
>
> Signed-off-by: Ruslan Bilovol <ruslan.bilovol@gmail.com>
> ---
> drivers/usb/gadget/function/f_uac2.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/gadget/function/f_uac2.c
> b/drivers/usb/gadget/function/f_uac2.c
> index 3633df6..fb9b875 100644
> --- a/drivers/usb/gadget/function/f_uac2.c
> +++ b/drivers/usb/gadget/function/f_uac2.c
> @@ -281,7 +281,7 @@ enum {
>
> .bmAttributes = USB_ENDPOINT_XFER_ISOC | USB_ENDPOINT_SYNC_ASYNC,
> .wMaxPacketSize = cpu_to_le16(1024),
> - .bInterval = 4,
> + .bInterval = 1,
> };
>
> /* CS AS ISO OUT Endpoint */
> @@ -358,7 +358,7 @@ enum {
>
> .bmAttributes = USB_ENDPOINT_XFER_ISOC | USB_ENDPOINT_SYNC_ASYNC,
> .wMaxPacketSize = cpu_to_le16(1024),
> - .bInterval = 4,
> + .bInterval = 1,
> };
>
> /* CS AS ISO IN Endpoint */
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 0/3] UAC2 Gadget: feedback endpoint support
2020-11-26 13:16 ` Jerome Brunet
@ 2020-11-26 13:44 ` Pavel Hofman
2020-12-04 14:39 ` Ruslan Bilovol
2020-12-04 14:35 ` Ruslan Bilovol
1 sibling, 1 reply; 31+ messages in thread
From: Pavel Hofman @ 2020-11-26 13:44 UTC (permalink / raw)
To: Jerome Brunet, Ruslan Bilovol, Glenn Schmottlach
Cc: Peter Chen, balbi, linux-usb
Dne 26. 11. 20 v 14:16 Jerome Brunet napsal(a):
>
>> Maximum data rates that I used were (AFAIR) 8 channel 192kHz/32bit,
>> but there is another issue with high data rate if someone uses so many
>> channels, very high sampling frequency or sample size that data can't
>> fit into allowed (by USB spec) max packet size of endpoint. In this case
>> need to decrease bInterval of endpoint.
Should anyone test the patches with RPi4 dwc2 as the gadget, please note
the recently accepted patch
https://patchwork.kernel.org/project/linux-usb/patch/e9e7d070-593c-122f-3a5c-2435bb147ab2@ivitera.com/
which allows using full 1024 bytes maxpacketsize on EP OUT. It is not an
enumeration issue, but the old (= existing) RX FIFO size drops data with
packet sizes above 960 bytes.
Thanks a lot for the great work.
Best regards,
Pavel.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 0/3] UAC2 Gadget: feedback endpoint support
2020-11-25 19:28 ` Glenn Schmottlach
@ 2020-11-28 23:26 ` Ruslan Bilovol
2020-12-01 21:43 ` Glenn Schmottlach
0 siblings, 1 reply; 31+ messages in thread
From: Ruslan Bilovol @ 2020-11-28 23:26 UTC (permalink / raw)
To: Glenn Schmottlach; +Cc: Peter Chen, balbi, linux-usb
On Wed, Nov 25, 2020 at 9:29 PM Glenn Schmottlach
<gschmottlach@gmail.com> wrote:
>
> On Sun, Nov 22, 2020 at 2:52 PM Ruslan Bilovol <ruslan.bilovol@gmail.com> wrote:
> >
> > On Fri, Nov 13, 2020 at 5:35 PM Glenn Schmottlach
> > <gschmottlach@gmail.com> wrote:
> > >
> > > On Thu, Nov 12, 2020 at 6:20 PM Ruslan Bilovol <ruslan.bilovol@gmail.com> wrote:
> > > >
> > > > On Wed, Nov 11, 2020 at 11:30 AM Peter Chen <peter.chen@nxp.com> wrote:
> > > > >
> > > > > On 20-11-08 02:18:28, Ruslan Bilovol wrote:
> > > > > > Current UAC2 gadget implements capture/sync paths
> > > > > > as two USB ISO ASYNC endpoints (IN and OUT).
> > > > > >
> > > > > > This violates USB spec which says that ISO ASYNC OUT endpoint
> > > > > > should have feedback companion endpoint.
> > > > > > See USB2.0 spec "5.12.4.1 Synchronization Type": asynchronous
> > > > > > sink provides explicit feedback (isochronous pipe).
> > > > > > Interesting that for ISO ASYNC *IN* endpoint respective
> > > > > > feedback isn't required since source provides implicit
> > > > > > feedforward (data stream).
> > > > > >
> > > > > > While it's not an issue if UAC2 Gadget is connected to
> > > > > > Linux host (Linux ignores missing feedback endpoint),
> > > > > > with other hosts like Windows or MacOS the UAC2 Gadget
> > > > > > isn't enumerated due to missing feedback endpoint.
> > > > > >
> > > > > > This patch series adds feedback endpoint support to
> > > > > > UAC2 function, new control to UAC2 mixer which can
> > > > > > be used by userspace tools (like alsaloop from alsa-utils)
> > > > > > for updating feedback frequency reported to the host.
> > > > > > This is useful for usecases when UAC2 Gadget's audio
> > > > > > samples are played to another codec or audio card
> > > > > > with its own internal freerunning clock so host can
> > > > > > be notified that more/less samples are required.
> > > > > >
> > > > > > The alsaloop tool requires some (relatively small)
> > > > > > modifications in order to start support driving
> > > > > > feedback frequency through UAC2 mixer control.
> > > > > > That change will be sent as a separate patch
> > > > > > to ALSA community.
> > > > > >
> > > > > > Also added ability to switch ISO ASYNC OUT endpoint into
> > > > > > adaptive endpoint which doesn't require feedback endpoint
> > > > > > (as per USB spec).
> > > > > >
> > > > > > Ruslan Bilovol (3):
> > > > > > usb: gadget: f_uac2/u_audio: add feedback endpoint support
> > > > > > usb: gadget: f_uac2: add adaptive sync support for capture
> > > > > > usb: gadget: u_audio: add real feedback implementation
> > > > >
> > > > > Hi Ruslan,
> > > > >
> > > > > I applied your patches, but WIN10 still can't recognize it well.
> > > > > The UAC1 is OK for WIN10 with the below same configuration.
> > > > > Any debug information you would like to know to check it?
> > > > >
> > > > >
> > > > > if [ "$FUNC" == "uac2" ]; then
> > > > > mkdir functions/$FUNC".0"
> > > > > echo 2 > functions/$FUNC".0"/p_ssize
> > > > > echo 48000 > functions/$FUNC".0"/p_srate
> > > > > echo 3 > functions/$FUNC".0"/p_chmask
> > > > >
> > > > > echo 2 > functions/$FUNC".0"/c_ssize
> > > > > echo 48000 > functions/$FUNC".0"/c_srate
> > > > > echo 3 > functions/$FUNC".0"/c_chmask
> > > > > #echo 4 > functions/$FUNC".0"/req_number
> > > > > ln -s functions/$FUNC".0" configs/c.1
> > > > > echo high-speed > /sys/kernel/config/usb_gadget/g1/max_speed
> > > > > fi
> > > > >
> > > >
> > > > Hmm... I just tested below config and it works fine with my Win10.
> > > > The only thing I noticed is Windows doesn't enable UAC2 gadget
> > > > by default, but this can be done from Win10 sound settings
> > > >
> > > > --------------------------------->8--------------------------------------
> > > > mkdir cfg
> > > > mount none cfg -t configfs
> > > > mkdir cfg/usb_gadget/g1
> > > > cd cfg/usb_gadget/g1
> > > > mkdir configs/c.1
> > > > mkdir functions/uac2.0
> > > >
> > > > # 44.1 kHz sample rate
> > > > echo 44100 > functions/uac2.0/c_srate
> > > > echo 44100 > functions/uac2.0/p_srate
> > > >
> > > > mkdir strings/0x409
> > > > mkdir configs/c.1/strings/0x409
> > > > echo 0x0101 > idProduct
> > > > echo 0x1d6b > idVendor
> > > > echo my-serial-num > strings/0x409/serialnumber
> > > > echo my-manufacturer > strings/0x409/manufacturer
> > > > echo "Test gadget" > strings/0x409/product
> > > > echo "Conf 1" > configs/c.1/strings/0x409/configuration
> > > > echo 120 > configs/c.1/MaxPower
> > > > ln -s functions/uac2.0 configs/c.1
> > > > echo musb-hdrc.0 > UDC
> > > > --------------------------------->8--------------------------------------
> > > >
> > > > Thanks,
> > > > Ruslan
> > >
> > > Hi Ruslan -
> > >
> > > With your configuration (above) Win10 was able to recognize and load
> > > the driver. What appears to prevent my configuration from loading is
> > > the fact that I selected 48K as my sample rate and my capture/playback
> > > channel mask includes more than two (2) channels. If I take your
> > > configuration and change the sample rate (c_srate/p_srate) to 48000
> > > Windows will fail to load the driver. Likewise, setting the
> > > c_chmask/p_chmask to 7 (three channels) will also cause the driver to
> > > fail to load.
> > >
> > > You mentioned there is an option in Win10 Sound Settings to "enable"
> > > UAC2 by default. I cannot find that option and I wonder if this is
> > > what is preventing me from changing the sample rate or channel mask?
> > > Could Windows be treating my audio gadget as a UAC1 device rather than
> > > a fully multi-channel audio device (even though the usbaudio2.sys
> > > driver is loaded)? Have you tried other configurations to verify your
> > > Win10 box supports more than two channels and a 44.1K sample rate? I
> > > look forward to your feedback and any suggestions you might offer.
> > >
> >
> > I was able to reproduce the issue and prepared a patch below, please
> > try it and see if it fixes the issue.
> >
> > Maximum data rates that I used were (AFAIR) 8 channel 192kHz/32bit,
> > but there is another issue with high data rate if someone uses so many
> > channels, very high sampling frequency or sample size that data can't
> > fit into allowed (by USB spec) max packet size of endpoint. In this case
> > need to decrease bInterval of endpoint.
> > I test it in a high-performance audio case so always use a minimal possible
> > bInterval (set to '1').
> > Of course in the future that need to be calculated dynamically depending on
> > the UAC2 settings
> >
> > Please test patches below and see it it helps
> >
> > ---------------------------------------------8<----------------------------------------
> > From 51516435bbf2486574ec7bc9fd4726677cd474a4 Mon Sep 17 00:00:00 2001
> > From: Ruslan Bilovol <ruslan.bilovol@gmail.com>
> > Date: Sun, 22 Nov 2020 21:05:38 +0200
> > Subject: [PATCH] usb: gadget: f_uac2: always increase endpoint max_packet_size
> > by one audio slot
> >
> > As per UAC2 Audio Data Formats spec (2.3.1.1 USB Packets),
> > if the sampling rate is a constant, the allowable variation
> > of number of audio slots per virtual frame is +/- 1 audio slot.
> >
> > It means that endpoint should be able to accept/send +1 audio
> > slot.
> >
> > Previous endpoint max_packet_size calculation code
> > was adding sometimes +1 audio slot due to DIV_ROUND_UP
> > behaviour which was rounding up to closest integer.
> > However this doesn't work if the numbers are divisible.
> >
> > It had no any impact with Linux hosts which ignore
> > this issue, but in case of more strict Windows it
> > caused rejected enumeration
> >
> > Thus always add +1 audio slot to endpoint's max packet size
> >
> > Signed-off-by: Ruslan Bilovol <ruslan.bilovol@gmail.com>
> > ---
> > drivers/usb/gadget/function/f_uac2.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/usb/gadget/function/f_uac2.c
> > b/drivers/usb/gadget/function/f_uac2.c
> > index ebdb2a1..1698587 100644
> > --- a/drivers/usb/gadget/function/f_uac2.c
> > +++ b/drivers/usb/gadget/function/f_uac2.c
> > @@ -486,7 +486,7 @@ static void set_ep_max_packet_size(const struct
> > f_uac2_opts *uac2_opts,
> > }
> >
> > max_packet_size = num_channels(chmask) * ssize *
> > - DIV_ROUND_UP(srate, factor / (1 << (ep_desc->bInterval - 1)));
> > + ((srate / (factor / (1 << (ep_desc->bInterval - 1)))) + 1);
> >
> > if (!is_playback && (uac2_opts->c_sync == USB_ENDPOINT_SYNC_ASYNC))
> > max_packet_size = max_packet_size * FBACK_FREQ_MAX / 100;
> > --
> > 1.9.1
> >
> >
> > ---------------------------------------------8<----------------------------------------
> > From c8f2f2b414af672ec40841e75fb1ea761ae29122 Mon Sep 17 00:00:00 2001
> > From: Ruslan Bilovol <ruslan.bilovol@gmail.com>
> > Date: Sun, 16 Feb 2020 22:40:23 +0200
> > Subject: [PATCH] usb: gadget: f_uac2: change bInterval to 1 for
> > 8ch/24bit/44.1kHz
> >
> > With bInterval=4 one audio slot in case of 8ch/24bit/44.1kHz
> > exeeds MaxPacket size of ISO endpoint (>1024) and can't be
> > transferred in time.
> > USB spec ("5.6.3 Isochronous Transfer Packet Size Constraints")
> > says if we need to transfer more than 1024 it is a high speed, high
> > bandwidth endpoint and should have bInterval=1.
> >
> > In the future, bInterval should be dynamically calculated
> > depending on bandwith requirementes
> >
> > Signed-off-by: Ruslan Bilovol <ruslan.bilovol@gmail.com>
> > ---
> > drivers/usb/gadget/function/f_uac2.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/usb/gadget/function/f_uac2.c
> > b/drivers/usb/gadget/function/f_uac2.c
> > index 3633df6..fb9b875 100644
> > --- a/drivers/usb/gadget/function/f_uac2.c
> > +++ b/drivers/usb/gadget/function/f_uac2.c
> > @@ -281,7 +281,7 @@ enum {
> >
> > .bmAttributes = USB_ENDPOINT_XFER_ISOC | USB_ENDPOINT_SYNC_ASYNC,
> > .wMaxPacketSize = cpu_to_le16(1024),
> > - .bInterval = 4,
> > + .bInterval = 1,
> > };
> >
> > /* CS AS ISO OUT Endpoint */
> > @@ -358,7 +358,7 @@ enum {
> >
> > .bmAttributes = USB_ENDPOINT_XFER_ISOC | USB_ENDPOINT_SYNC_ASYNC,
> > .wMaxPacketSize = cpu_to_le16(1024),
> > - .bInterval = 4,
> > + .bInterval = 1,
> > };
> >
> > /* CS AS ISO IN Endpoint */
> > --
> > 1.9.1
> >
> >
> > ---------------------------------------------8<----------------------------------------
> >
> > Thanks,
> > Ruslan
>
> Hi Ruslan -
>
> I have had some mixed success with your additional patches. First, the
That's good news, thanks for testing!
> UAC2 gadget does correctly register with Windows 10 but only for 1-31
> channels. For a fully specified channel map (32 channels):
>
> c_chmask = 0xFFFFFFFF ( Does not load)
> p_chmask = 0xFFFFFFFF ( Does not load)
>
> the driver fails to load (code=10). This seems similar to the prior
> problem. Could there be another off-by-one problem here?
>
> With a 31 channel mask the Windows 10 usbaudio2.sys loads successfully, e.g.
>
> c_chmask = 0x7FFFFFFF (loads)
> p_chmask = 0x7FFFFFFF (loads)
That's correct behavior. UAC2 spec says that bits D27..D30 are reserved.
Also bit D31 is mutually exclusive with other bits and means 'raw data'.
It means that with current UAC2 gadget implementation you can enable
only 27 channels.
It seems like it's possible to add more channels by adding more audio
clusters but that requires UAC2 driver modifications. Also it's not clear
whether it's implemented in Host OS drivers
I personally tested as many as 8x8 playback/capture channels simultaneously.
>
> Once Windows 10 loads the driver successfully, however, I can't seem
> to select the device through Audacity on Windows. The device appears
> in the Windows "Device Manager" under "Sound, video and game
> controllers -> Source/Sink". How are you testing audio
> playback/capture under Windows? Perhaps my test approach is wrong?
You need to go to sound settings -> manage sound devices and enable
UAC2 gadget there, after that it will appear in the system.
I also used Audacity for testing this and it works fine for me
Thanks,
Ruslan
>
> I used the same approach on my Linux host where the USB UAC2 device
> appears as an ALSA device and I use speaker-test on the target
> (BeagleBone Black) to play and then capture the audio using Audacity
> on the Linux host. I was hoping to do the same or something similar
> under Windows.
>
> Below is the script I've been using on the target to configure my UAC2 device:
>
> mkdir -p "/sys/kernel/config/usb_gadget/g_multi"
> echo "${idVendor:-0x11F5}" > "/sys/kernel/config/usb_gadget/g_multi/idVendor"
> echo "${idProduct:-0x0009}" > "/sys/kernel/config/usb_gadget/g_multi/idProduct"
> echo "${bcdDevice:-0x0001}" > "/sys/kernel/config/usb_gadget/g_multi/bcdDevice"
> echo "${bDeviceClass:-0x00}" >
> "/sys/kernel/config/usb_gadget/g_multi/bDeviceClass"
> echo "${bDeviceSubClass:-0x00}" >
> "/sys/kernel/config/usb_gadget/g_multi/bDeviceSubClass"
> echo "${bDeviceProtocol:-0x00}" >
> "/sys/kernel/config/usb_gadget/g_multi/bDeviceProtocol"
> echo "${bcdUSB:-0x0200}" > "/sys/kernel/config/usb_gadget/g_multi/bcdUSB"
> echo "${bMaxPacketSize0:-0x00}" >
> "/sys/kernel/config/usb_gadget/g_multi/bMaxPacketSize0"
> mkdir -p "/sys/kernel/config/usb_gadget/g_multi/strings/0x409"
> echo "${serialnumber:-000000000000}" >
> "/sys/kernel/config/usb_gadget/g_multi/strings/0x409/serialnumber"
> echo "${product:-UAC2}" >
> "/sys/kernel/config/usb_gadget/g_multi/strings/0x409/product"
> echo "${manufacturer:-www.acme.com}" >
> "/sys/kernel/config/usb_gadget/g_multi/strings/0x409/manufacturer"
> mkdir -p "/sys/kernel/config/usb_gadget/g_multi/functions/uac2.usb0"
> echo "${req_number:-4}" >
> "/sys/kernel/config/usb_gadget/g_multi/functions/uac2.usb0/req_number"
> echo "${c_ssize:-4}" >
> "/sys/kernel/config/usb_gadget/g_multi/functions/uac2.usb0/c_ssize"
> echo "${c_srate:-48000}" >
> "/sys/kernel/config/usb_gadget/g_multi/functions/uac2.usb0/c_srate"
> echo "${c_chmask:-0x00000003}" >
> "/sys/kernel/config/usb_gadget/g_multi/functions/uac2.usb0/c_chmask"
> echo "${c_sync:-asynchronous}" >
> "/sys/kernel/config/usb_gadget/g_multi/functions/uac2.usb0/c_sync"
> echo "${p_ssize:-4}" >
> "/sys/kernel/config/usb_gadget/g_multi/functions/uac2.usb0/p_ssize"
> echo "${p_srate:-48000}" >
> "/sys/kernel/config/usb_gadget/g_multi/functions/uac2.usb0/p_srate"
> echo "${p_chmask:-0x00000003}" >
> "/sys/kernel/config/usb_gadget/g_multi/functions/uac2.usb0/p_chmask"
> mkdir -p "/sys/kernel/config/usb_gadget/g_multi/configs/c.1"
> echo "${bmAttributes:-0x80}" >
> "/sys/kernel/config/usb_gadget/g_multi/configs/c.1/bmAttributes"
> echo "${MaxPower:-500}" >
> "/sys/kernel/config/usb_gadget/g_multi/configs/c.1/MaxPower"
> mkdir -p "/sys/kernel/config/usb_gadget/g_multi/configs/c.1/strings/0x409"
> echo "${configuration:-BeagleBone Composite}" >
> "/sys/kernel/config/usb_gadget/g_multi/configs/c.1/strings/0x409/configuration"
> ln -s "/sys/kernel/config/usb_gadget/g_multi/functions/uac2.usb0"
> "/sys/kernel/config/usb_gadget/g_multi/configs/c.1/uac2.usb0"
> mkdir -p "/sys/kernel/config/usb_gadget/g_multi/os_desc"
> ln -s "//sys/kernel/config/usb_gadget/g_multi/configs/c.1"
> "/sys/kernel/config/usb_gadget/g_multi/os_desc/c.1"
> echo "${qw_sign:-MSFT100}" >
> "/sys/kernel/config/usb_gadget/g_multi/os_desc/qw_sign"
> echo "${b_vendor_code:-0x00}" >
> "/sys/kernel/config/usb_gadget/g_multi/os_desc/b_vendor_code"
> echo "${use:-1}" > "/sys/kernel/config/usb_gadget/g_multi/os_desc/use"
>
> basename /sys/class/udc/* > /sys/kernel/config/usb_gadget/g_multi/UDC
>
> This is only a two channel playback/capture configuration. I hoped to
> get this working under Windows before increasing my channel count to a
> full 32 channel mask with a 48K sample rate.
>
> I look forward to hearing your suggestions and possibly pointing out
> where I've gone astray.
>
> Thanks,
>
> Glenn
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 0/3] UAC2 Gadget: feedback endpoint support
2020-11-28 23:26 ` Ruslan Bilovol
@ 2020-12-01 21:43 ` Glenn Schmottlach
2020-12-02 22:04 ` Glenn Schmottlach
2020-12-10 12:46 ` Ruslan Bilovol
0 siblings, 2 replies; 31+ messages in thread
From: Glenn Schmottlach @ 2020-12-01 21:43 UTC (permalink / raw)
To: Ruslan Bilovol; +Cc: Peter Chen, balbi, linux-usb
On Sat, Nov 28, 2020 at 6:26 PM Ruslan Bilovol <ruslan.bilovol@gmail.com> wrote:
>
> On Wed, Nov 25, 2020 at 9:29 PM Glenn Schmottlach
> <gschmottlach@gmail.com> wrote:
> >
> > On Sun, Nov 22, 2020 at 2:52 PM Ruslan Bilovol <ruslan.bilovol@gmail.com> wrote:
> > >
> > > On Fri, Nov 13, 2020 at 5:35 PM Glenn Schmottlach
> > > <gschmottlach@gmail.com> wrote:
> > > >
> > > > On Thu, Nov 12, 2020 at 6:20 PM Ruslan Bilovol <ruslan.bilovol@gmail.com> wrote:
> > > > >
> > > > > On Wed, Nov 11, 2020 at 11:30 AM Peter Chen <peter.chen@nxp.com> wrote:
> > > > > >
> > > > > > On 20-11-08 02:18:28, Ruslan Bilovol wrote:
> > > > > > > Current UAC2 gadget implements capture/sync paths
> > > > > > > as two USB ISO ASYNC endpoints (IN and OUT).
> > > > > > >
> > > > > > > This violates USB spec which says that ISO ASYNC OUT endpoint
> > > > > > > should have feedback companion endpoint.
> > > > > > > See USB2.0 spec "5.12.4.1 Synchronization Type": asynchronous
> > > > > > > sink provides explicit feedback (isochronous pipe).
> > > > > > > Interesting that for ISO ASYNC *IN* endpoint respective
> > > > > > > feedback isn't required since source provides implicit
> > > > > > > feedforward (data stream).
> > > > > > >
> > > > > > > While it's not an issue if UAC2 Gadget is connected to
> > > > > > > Linux host (Linux ignores missing feedback endpoint),
> > > > > > > with other hosts like Windows or MacOS the UAC2 Gadget
> > > > > > > isn't enumerated due to missing feedback endpoint.
> > > > > > >
> > > > > > > This patch series adds feedback endpoint support to
> > > > > > > UAC2 function, new control to UAC2 mixer which can
> > > > > > > be used by userspace tools (like alsaloop from alsa-utils)
> > > > > > > for updating feedback frequency reported to the host.
> > > > > > > This is useful for usecases when UAC2 Gadget's audio
> > > > > > > samples are played to another codec or audio card
> > > > > > > with its own internal freerunning clock so host can
> > > > > > > be notified that more/less samples are required.
> > > > > > >
> > > > > > > The alsaloop tool requires some (relatively small)
> > > > > > > modifications in order to start support driving
> > > > > > > feedback frequency through UAC2 mixer control.
> > > > > > > That change will be sent as a separate patch
> > > > > > > to ALSA community.
> > > > > > >
> > > > > > > Also added ability to switch ISO ASYNC OUT endpoint into
> > > > > > > adaptive endpoint which doesn't require feedback endpoint
> > > > > > > (as per USB spec).
> > > > > > >
> > > > > > > Ruslan Bilovol (3):
> > > > > > > usb: gadget: f_uac2/u_audio: add feedback endpoint support
> > > > > > > usb: gadget: f_uac2: add adaptive sync support for capture
> > > > > > > usb: gadget: u_audio: add real feedback implementation
> > > > > >
> > > > > > Hi Ruslan,
> > > > > >
> > > > > > I applied your patches, but WIN10 still can't recognize it well.
> > > > > > The UAC1 is OK for WIN10 with the below same configuration.
> > > > > > Any debug information you would like to know to check it?
> > > > > >
> > > > > >
> > > > > > if [ "$FUNC" == "uac2" ]; then
> > > > > > mkdir functions/$FUNC".0"
> > > > > > echo 2 > functions/$FUNC".0"/p_ssize
> > > > > > echo 48000 > functions/$FUNC".0"/p_srate
> > > > > > echo 3 > functions/$FUNC".0"/p_chmask
> > > > > >
> > > > > > echo 2 > functions/$FUNC".0"/c_ssize
> > > > > > echo 48000 > functions/$FUNC".0"/c_srate
> > > > > > echo 3 > functions/$FUNC".0"/c_chmask
> > > > > > #echo 4 > functions/$FUNC".0"/req_number
> > > > > > ln -s functions/$FUNC".0" configs/c.1
> > > > > > echo high-speed > /sys/kernel/config/usb_gadget/g1/max_speed
> > > > > > fi
> > > > > >
> > > > >
> > > > > Hmm... I just tested below config and it works fine with my Win10.
> > > > > The only thing I noticed is Windows doesn't enable UAC2 gadget
> > > > > by default, but this can be done from Win10 sound settings
> > > > >
> > > > > --------------------------------->8--------------------------------------
> > > > > mkdir cfg
> > > > > mount none cfg -t configfs
> > > > > mkdir cfg/usb_gadget/g1
> > > > > cd cfg/usb_gadget/g1
> > > > > mkdir configs/c.1
> > > > > mkdir functions/uac2.0
> > > > >
> > > > > # 44.1 kHz sample rate
> > > > > echo 44100 > functions/uac2.0/c_srate
> > > > > echo 44100 > functions/uac2.0/p_srate
> > > > >
> > > > > mkdir strings/0x409
> > > > > mkdir configs/c.1/strings/0x409
> > > > > echo 0x0101 > idProduct
> > > > > echo 0x1d6b > idVendor
> > > > > echo my-serial-num > strings/0x409/serialnumber
> > > > > echo my-manufacturer > strings/0x409/manufacturer
> > > > > echo "Test gadget" > strings/0x409/product
> > > > > echo "Conf 1" > configs/c.1/strings/0x409/configuration
> > > > > echo 120 > configs/c.1/MaxPower
> > > > > ln -s functions/uac2.0 configs/c.1
> > > > > echo musb-hdrc.0 > UDC
> > > > > --------------------------------->8--------------------------------------
> > > > >
> > > > > Thanks,
> > > > > Ruslan
> > > >
> > > > Hi Ruslan -
> > > >
> > > > With your configuration (above) Win10 was able to recognize and load
> > > > the driver. What appears to prevent my configuration from loading is
> > > > the fact that I selected 48K as my sample rate and my capture/playback
> > > > channel mask includes more than two (2) channels. If I take your
> > > > configuration and change the sample rate (c_srate/p_srate) to 48000
> > > > Windows will fail to load the driver. Likewise, setting the
> > > > c_chmask/p_chmask to 7 (three channels) will also cause the driver to
> > > > fail to load.
> > > >
> > > > You mentioned there is an option in Win10 Sound Settings to "enable"
> > > > UAC2 by default. I cannot find that option and I wonder if this is
> > > > what is preventing me from changing the sample rate or channel mask?
> > > > Could Windows be treating my audio gadget as a UAC1 device rather than
> > > > a fully multi-channel audio device (even though the usbaudio2.sys
> > > > driver is loaded)? Have you tried other configurations to verify your
> > > > Win10 box supports more than two channels and a 44.1K sample rate? I
> > > > look forward to your feedback and any suggestions you might offer.
> > > >
> > >
> > > I was able to reproduce the issue and prepared a patch below, please
> > > try it and see if it fixes the issue.
> > >
> > > Maximum data rates that I used were (AFAIR) 8 channel 192kHz/32bit,
> > > but there is another issue with high data rate if someone uses so many
> > > channels, very high sampling frequency or sample size that data can't
> > > fit into allowed (by USB spec) max packet size of endpoint. In this case
> > > need to decrease bInterval of endpoint.
> > > I test it in a high-performance audio case so always use a minimal possible
> > > bInterval (set to '1').
> > > Of course in the future that need to be calculated dynamically depending on
> > > the UAC2 settings
> > >
> > > Please test patches below and see it it helps
> > >
> > > ---------------------------------------------8<----------------------------------------
> > > From 51516435bbf2486574ec7bc9fd4726677cd474a4 Mon Sep 17 00:00:00 2001
> > > From: Ruslan Bilovol <ruslan.bilovol@gmail.com>
> > > Date: Sun, 22 Nov 2020 21:05:38 +0200
> > > Subject: [PATCH] usb: gadget: f_uac2: always increase endpoint max_packet_size
> > > by one audio slot
> > >
> > > As per UAC2 Audio Data Formats spec (2.3.1.1 USB Packets),
> > > if the sampling rate is a constant, the allowable variation
> > > of number of audio slots per virtual frame is +/- 1 audio slot.
> > >
> > > It means that endpoint should be able to accept/send +1 audio
> > > slot.
> > >
> > > Previous endpoint max_packet_size calculation code
> > > was adding sometimes +1 audio slot due to DIV_ROUND_UP
> > > behaviour which was rounding up to closest integer.
> > > However this doesn't work if the numbers are divisible.
> > >
> > > It had no any impact with Linux hosts which ignore
> > > this issue, but in case of more strict Windows it
> > > caused rejected enumeration
> > >
> > > Thus always add +1 audio slot to endpoint's max packet size
> > >
> > > Signed-off-by: Ruslan Bilovol <ruslan.bilovol@gmail.com>
> > > ---
> > > drivers/usb/gadget/function/f_uac2.c | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/usb/gadget/function/f_uac2.c
> > > b/drivers/usb/gadget/function/f_uac2.c
> > > index ebdb2a1..1698587 100644
> > > --- a/drivers/usb/gadget/function/f_uac2.c
> > > +++ b/drivers/usb/gadget/function/f_uac2.c
> > > @@ -486,7 +486,7 @@ static void set_ep_max_packet_size(const struct
> > > f_uac2_opts *uac2_opts,
> > > }
> > >
> > > max_packet_size = num_channels(chmask) * ssize *
> > > - DIV_ROUND_UP(srate, factor / (1 << (ep_desc->bInterval - 1)));
> > > + ((srate / (factor / (1 << (ep_desc->bInterval - 1)))) + 1);
> > >
> > > if (!is_playback && (uac2_opts->c_sync == USB_ENDPOINT_SYNC_ASYNC))
> > > max_packet_size = max_packet_size * FBACK_FREQ_MAX / 100;
> > > --
> > > 1.9.1
> > >
> > >
> > > ---------------------------------------------8<----------------------------------------
> > > From c8f2f2b414af672ec40841e75fb1ea761ae29122 Mon Sep 17 00:00:00 2001
> > > From: Ruslan Bilovol <ruslan.bilovol@gmail.com>
> > > Date: Sun, 16 Feb 2020 22:40:23 +0200
> > > Subject: [PATCH] usb: gadget: f_uac2: change bInterval to 1 for
> > > 8ch/24bit/44.1kHz
> > >
> > > With bInterval=4 one audio slot in case of 8ch/24bit/44.1kHz
> > > exeeds MaxPacket size of ISO endpoint (>1024) and can't be
> > > transferred in time.
> > > USB spec ("5.6.3 Isochronous Transfer Packet Size Constraints")
> > > says if we need to transfer more than 1024 it is a high speed, high
> > > bandwidth endpoint and should have bInterval=1.
> > >
> > > In the future, bInterval should be dynamically calculated
> > > depending on bandwith requirementes
> > >
> > > Signed-off-by: Ruslan Bilovol <ruslan.bilovol@gmail.com>
> > > ---
> > > drivers/usb/gadget/function/f_uac2.c | 4 ++--
> > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/usb/gadget/function/f_uac2.c
> > > b/drivers/usb/gadget/function/f_uac2.c
> > > index 3633df6..fb9b875 100644
> > > --- a/drivers/usb/gadget/function/f_uac2.c
> > > +++ b/drivers/usb/gadget/function/f_uac2.c
> > > @@ -281,7 +281,7 @@ enum {
> > >
> > > .bmAttributes = USB_ENDPOINT_XFER_ISOC | USB_ENDPOINT_SYNC_ASYNC,
> > > .wMaxPacketSize = cpu_to_le16(1024),
> > > - .bInterval = 4,
> > > + .bInterval = 1,
> > > };
> > >
> > > /* CS AS ISO OUT Endpoint */
> > > @@ -358,7 +358,7 @@ enum {
> > >
> > > .bmAttributes = USB_ENDPOINT_XFER_ISOC | USB_ENDPOINT_SYNC_ASYNC,
> > > .wMaxPacketSize = cpu_to_le16(1024),
> > > - .bInterval = 4,
> > > + .bInterval = 1,
> > > };
> > >
> > > /* CS AS ISO IN Endpoint */
> > > --
> > > 1.9.1
> > >
> > >
> > > ---------------------------------------------8<----------------------------------------
> > >
> > > Thanks,
> > > Ruslan
> >
> > Hi Ruslan -
> >
> > I have had some mixed success with your additional patches. First, the
>
> That's good news, thanks for testing!
>
> > UAC2 gadget does correctly register with Windows 10 but only for 1-31
> > channels. For a fully specified channel map (32 channels):
> >
> > c_chmask = 0xFFFFFFFF ( Does not load)
> > p_chmask = 0xFFFFFFFF ( Does not load)
> >
> > the driver fails to load (code=10). This seems similar to the prior
> > problem. Could there be another off-by-one problem here?
> >
> > With a 31 channel mask the Windows 10 usbaudio2.sys loads successfully, e.g.
> >
> > c_chmask = 0x7FFFFFFF (loads)
> > p_chmask = 0x7FFFFFFF (loads)
>
> That's correct behavior. UAC2 spec says that bits D27..D30 are reserved.
> Also bit D31 is mutually exclusive with other bits and means 'raw data'.
> It means that with current UAC2 gadget implementation you can enable
> only 27 channels.
>
> It seems like it's possible to add more channels by adding more audio
> clusters but that requires UAC2 driver modifications. Also it's not clear
> whether it's implemented in Host OS drivers
>
> I personally tested as many as 8x8 playback/capture channels simultaneously.
>
> >
> > Once Windows 10 loads the driver successfully, however, I can't seem
> > to select the device through Audacity on Windows. The device appears
> > in the Windows "Device Manager" under "Sound, video and game
> > controllers -> Source/Sink". How are you testing audio
> > playback/capture under Windows? Perhaps my test approach is wrong?
>
> You need to go to sound settings -> manage sound devices and enable
> UAC2 gadget there, after that it will appear in the system.
>
> I also used Audacity for testing this and it works fine for me
>
> Thanks,
> Ruslan
>
> >
> > I used the same approach on my Linux host where the USB UAC2 device
> > appears as an ALSA device and I use speaker-test on the target
> > (BeagleBone Black) to play and then capture the audio using Audacity
> > on the Linux host. I was hoping to do the same or something similar
> > under Windows.
> >
> > Below is the script I've been using on the target to configure my UAC2 device:
> >
> > mkdir -p "/sys/kernel/config/usb_gadget/g_multi"
> > echo "${idVendor:-0x11F5}" > "/sys/kernel/config/usb_gadget/g_multi/idVendor"
> > echo "${idProduct:-0x0009}" > "/sys/kernel/config/usb_gadget/g_multi/idProduct"
> > echo "${bcdDevice:-0x0001}" > "/sys/kernel/config/usb_gadget/g_multi/bcdDevice"
> > echo "${bDeviceClass:-0x00}" >
> > "/sys/kernel/config/usb_gadget/g_multi/bDeviceClass"
> > echo "${bDeviceSubClass:-0x00}" >
> > "/sys/kernel/config/usb_gadget/g_multi/bDeviceSubClass"
> > echo "${bDeviceProtocol:-0x00}" >
> > "/sys/kernel/config/usb_gadget/g_multi/bDeviceProtocol"
> > echo "${bcdUSB:-0x0200}" > "/sys/kernel/config/usb_gadget/g_multi/bcdUSB"
> > echo "${bMaxPacketSize0:-0x00}" >
> > "/sys/kernel/config/usb_gadget/g_multi/bMaxPacketSize0"
> > mkdir -p "/sys/kernel/config/usb_gadget/g_multi/strings/0x409"
> > echo "${serialnumber:-000000000000}" >
> > "/sys/kernel/config/usb_gadget/g_multi/strings/0x409/serialnumber"
> > echo "${product:-UAC2}" >
> > "/sys/kernel/config/usb_gadget/g_multi/strings/0x409/product"
> > echo "${manufacturer:-www.acme.com}" >
> > "/sys/kernel/config/usb_gadget/g_multi/strings/0x409/manufacturer"
> > mkdir -p "/sys/kernel/config/usb_gadget/g_multi/functions/uac2.usb0"
> > echo "${req_number:-4}" >
> > "/sys/kernel/config/usb_gadget/g_multi/functions/uac2.usb0/req_number"
> > echo "${c_ssize:-4}" >
> > "/sys/kernel/config/usb_gadget/g_multi/functions/uac2.usb0/c_ssize"
> > echo "${c_srate:-48000}" >
> > "/sys/kernel/config/usb_gadget/g_multi/functions/uac2.usb0/c_srate"
> > echo "${c_chmask:-0x00000003}" >
> > "/sys/kernel/config/usb_gadget/g_multi/functions/uac2.usb0/c_chmask"
> > echo "${c_sync:-asynchronous}" >
> > "/sys/kernel/config/usb_gadget/g_multi/functions/uac2.usb0/c_sync"
> > echo "${p_ssize:-4}" >
> > "/sys/kernel/config/usb_gadget/g_multi/functions/uac2.usb0/p_ssize"
> > echo "${p_srate:-48000}" >
> > "/sys/kernel/config/usb_gadget/g_multi/functions/uac2.usb0/p_srate"
> > echo "${p_chmask:-0x00000003}" >
> > "/sys/kernel/config/usb_gadget/g_multi/functions/uac2.usb0/p_chmask"
> > mkdir -p "/sys/kernel/config/usb_gadget/g_multi/configs/c.1"
> > echo "${bmAttributes:-0x80}" >
> > "/sys/kernel/config/usb_gadget/g_multi/configs/c.1/bmAttributes"
> > echo "${MaxPower:-500}" >
> > "/sys/kernel/config/usb_gadget/g_multi/configs/c.1/MaxPower"
> > mkdir -p "/sys/kernel/config/usb_gadget/g_multi/configs/c.1/strings/0x409"
> > echo "${configuration:-BeagleBone Composite}" >
> > "/sys/kernel/config/usb_gadget/g_multi/configs/c.1/strings/0x409/configuration"
> > ln -s "/sys/kernel/config/usb_gadget/g_multi/functions/uac2.usb0"
> > "/sys/kernel/config/usb_gadget/g_multi/configs/c.1/uac2.usb0"
> > mkdir -p "/sys/kernel/config/usb_gadget/g_multi/os_desc"
> > ln -s "//sys/kernel/config/usb_gadget/g_multi/configs/c.1"
> > "/sys/kernel/config/usb_gadget/g_multi/os_desc/c.1"
> > echo "${qw_sign:-MSFT100}" >
> > "/sys/kernel/config/usb_gadget/g_multi/os_desc/qw_sign"
> > echo "${b_vendor_code:-0x00}" >
> > "/sys/kernel/config/usb_gadget/g_multi/os_desc/b_vendor_code"
> > echo "${use:-1}" > "/sys/kernel/config/usb_gadget/g_multi/os_desc/use"
> >
> > basename /sys/class/udc/* > /sys/kernel/config/usb_gadget/g_multi/UDC
> >
> > This is only a two channel playback/capture configuration. I hoped to
> > get this working under Windows before increasing my channel count to a
> > full 32 channel mask with a 48K sample rate.
> >
> > I look forward to hearing your suggestions and possibly pointing out
> > where I've gone astray.
> >
> > Thanks,
> >
> > Glenn
Hi Ruslan -
Thanks for the feedback but unfortunately I've experienced mixed
results with the gadget UAC2 driver on both Windows/Linux. Let me
describe my environment. My host platform is either a Linux Ubuntu
18.04 or Windows 10 laptop while the target environment is a
BeagleBone Black (Linux beaglebone 5.4.74-g9574bba32a #1 PREEMPT). I'm
testing two different scenarios:
Scenario #1:
BeagleBone Black (BBB) runs speaker-test generating a single channel
(S32_LE) audio stream containing a 1KHz tone with a 48K sample rate,
e.g.
> speaker-test -D hw:1,0 -r 48000 -c 1 -f 1000 -F S32_LE -t sine
The host laptop is running Audacity and recording the tone over the
UAC2 adapter. On the Linux host the capture is correct and the tone is
bit-perfect. On the Windows 10 the capture contains numerous missing
samples which translates into a lot of audible pops and clicks.
Scenario #2:
The Linux/Windows host plays a single channel, 48K, S32_LE 1K sine
tone to the target using either Audacity (on Windows) or 'aplay' (on
Linux), e.g.
> aplay -D hw:4,0 -c 1 -r 48000 -t wav tone_1k.wav (Linux)
On the BBB target I use 'arecord' to record the tone to a RAM disk and
then copy the recorded file back to the host where I can verify the
quality of the recording. In both instances (e.g. using either Windows
or Linux for playback) the recording on the target results in a
captured file with missing samples and audible pops/clicks. In this
scenario the UAC2 gadget is configured with c_sync == asynchronous. I
wouldn't expect things to improve with c_sync == adaptive since you
mentioned in your patch that it always reports back the nominal
frequency to the host from the feedback endpoint.
Do you have any suggestions that might explain (the above) behavior.
Can you describe your test environment in more detail so that I can
perhaps re-create it? What Linux target are you using with your tests?
You mentioned you tested an 8x8 playback/capture scenario. Can you
provide any details of how you performed this test and the method you
used to confirm the audio quality for the capture/playback?
Thanks for any insights you might be able to offer . . .
Glenn
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 0/3] UAC2 Gadget: feedback endpoint support
2020-12-01 21:43 ` Glenn Schmottlach
@ 2020-12-02 22:04 ` Glenn Schmottlach
2020-12-03 10:09 ` Peter Chen
2020-12-10 12:46 ` Ruslan Bilovol
1 sibling, 1 reply; 31+ messages in thread
From: Glenn Schmottlach @ 2020-12-02 22:04 UTC (permalink / raw)
To: Ruslan Bilovol; +Cc: Peter Chen, balbi, linux-usb
On Tue, Dec 1, 2020 at 4:43 PM Glenn Schmottlach <gschmottlach@gmail.com> wrote:
>
> On Sat, Nov 28, 2020 at 6:26 PM Ruslan Bilovol <ruslan.bilovol@gmail.com> wrote:
> >
> > On Wed, Nov 25, 2020 at 9:29 PM Glenn Schmottlach
> > <gschmottlach@gmail.com> wrote:
> > >
> > > On Sun, Nov 22, 2020 at 2:52 PM Ruslan Bilovol <ruslan.bilovol@gmail.com> wrote:
> > > >
> > > > On Fri, Nov 13, 2020 at 5:35 PM Glenn Schmottlach
> > > > <gschmottlach@gmail.com> wrote:
> > > > >
> > > > > On Thu, Nov 12, 2020 at 6:20 PM Ruslan Bilovol <ruslan.bilovol@gmail.com> wrote:
> > > > > >
> > > > > > On Wed, Nov 11, 2020 at 11:30 AM Peter Chen <peter.chen@nxp.com> wrote:
> > > > > > >
> > > > > > > On 20-11-08 02:18:28, Ruslan Bilovol wrote:
> > > > > > > > Current UAC2 gadget implements capture/sync paths
> > > > > > > > as two USB ISO ASYNC endpoints (IN and OUT).
> > > > > > > >
> > > > > > > > This violates USB spec which says that ISO ASYNC OUT endpoint
> > > > > > > > should have feedback companion endpoint.
> > > > > > > > See USB2.0 spec "5.12.4.1 Synchronization Type": asynchronous
> > > > > > > > sink provides explicit feedback (isochronous pipe).
> > > > > > > > Interesting that for ISO ASYNC *IN* endpoint respective
> > > > > > > > feedback isn't required since source provides implicit
> > > > > > > > feedforward (data stream).
> > > > > > > >
> > > > > > > > While it's not an issue if UAC2 Gadget is connected to
> > > > > > > > Linux host (Linux ignores missing feedback endpoint),
> > > > > > > > with other hosts like Windows or MacOS the UAC2 Gadget
> > > > > > > > isn't enumerated due to missing feedback endpoint.
> > > > > > > >
> > > > > > > > This patch series adds feedback endpoint support to
> > > > > > > > UAC2 function, new control to UAC2 mixer which can
> > > > > > > > be used by userspace tools (like alsaloop from alsa-utils)
> > > > > > > > for updating feedback frequency reported to the host.
> > > > > > > > This is useful for usecases when UAC2 Gadget's audio
> > > > > > > > samples are played to another codec or audio card
> > > > > > > > with its own internal freerunning clock so host can
> > > > > > > > be notified that more/less samples are required.
> > > > > > > >
> > > > > > > > The alsaloop tool requires some (relatively small)
> > > > > > > > modifications in order to start support driving
> > > > > > > > feedback frequency through UAC2 mixer control.
> > > > > > > > That change will be sent as a separate patch
> > > > > > > > to ALSA community.
> > > > > > > >
> > > > > > > > Also added ability to switch ISO ASYNC OUT endpoint into
> > > > > > > > adaptive endpoint which doesn't require feedback endpoint
> > > > > > > > (as per USB spec).
> > > > > > > >
> > > > > > > > Ruslan Bilovol (3):
> > > > > > > > usb: gadget: f_uac2/u_audio: add feedback endpoint support
> > > > > > > > usb: gadget: f_uac2: add adaptive sync support for capture
> > > > > > > > usb: gadget: u_audio: add real feedback implementation
> > > > > > >
> > > > > > > Hi Ruslan,
> > > > > > >
> > > > > > > I applied your patches, but WIN10 still can't recognize it well.
> > > > > > > The UAC1 is OK for WIN10 with the below same configuration.
> > > > > > > Any debug information you would like to know to check it?
> > > > > > >
> > > > > > >
> > > > > > > if [ "$FUNC" == "uac2" ]; then
> > > > > > > mkdir functions/$FUNC".0"
> > > > > > > echo 2 > functions/$FUNC".0"/p_ssize
> > > > > > > echo 48000 > functions/$FUNC".0"/p_srate
> > > > > > > echo 3 > functions/$FUNC".0"/p_chmask
> > > > > > >
> > > > > > > echo 2 > functions/$FUNC".0"/c_ssize
> > > > > > > echo 48000 > functions/$FUNC".0"/c_srate
> > > > > > > echo 3 > functions/$FUNC".0"/c_chmask
> > > > > > > #echo 4 > functions/$FUNC".0"/req_number
> > > > > > > ln -s functions/$FUNC".0" configs/c.1
> > > > > > > echo high-speed > /sys/kernel/config/usb_gadget/g1/max_speed
> > > > > > > fi
> > > > > > >
> > > > > >
> > > > > > Hmm... I just tested below config and it works fine with my Win10.
> > > > > > The only thing I noticed is Windows doesn't enable UAC2 gadget
> > > > > > by default, but this can be done from Win10 sound settings
> > > > > >
> > > > > > --------------------------------->8--------------------------------------
> > > > > > mkdir cfg
> > > > > > mount none cfg -t configfs
> > > > > > mkdir cfg/usb_gadget/g1
> > > > > > cd cfg/usb_gadget/g1
> > > > > > mkdir configs/c.1
> > > > > > mkdir functions/uac2.0
> > > > > >
> > > > > > # 44.1 kHz sample rate
> > > > > > echo 44100 > functions/uac2.0/c_srate
> > > > > > echo 44100 > functions/uac2.0/p_srate
> > > > > >
> > > > > > mkdir strings/0x409
> > > > > > mkdir configs/c.1/strings/0x409
> > > > > > echo 0x0101 > idProduct
> > > > > > echo 0x1d6b > idVendor
> > > > > > echo my-serial-num > strings/0x409/serialnumber
> > > > > > echo my-manufacturer > strings/0x409/manufacturer
> > > > > > echo "Test gadget" > strings/0x409/product
> > > > > > echo "Conf 1" > configs/c.1/strings/0x409/configuration
> > > > > > echo 120 > configs/c.1/MaxPower
> > > > > > ln -s functions/uac2.0 configs/c.1
> > > > > > echo musb-hdrc.0 > UDC
> > > > > > --------------------------------->8--------------------------------------
> > > > > >
> > > > > > Thanks,
> > > > > > Ruslan
> > > > >
> > > > > Hi Ruslan -
> > > > >
> > > > > With your configuration (above) Win10 was able to recognize and load
> > > > > the driver. What appears to prevent my configuration from loading is
> > > > > the fact that I selected 48K as my sample rate and my capture/playback
> > > > > channel mask includes more than two (2) channels. If I take your
> > > > > configuration and change the sample rate (c_srate/p_srate) to 48000
> > > > > Windows will fail to load the driver. Likewise, setting the
> > > > > c_chmask/p_chmask to 7 (three channels) will also cause the driver to
> > > > > fail to load.
> > > > >
> > > > > You mentioned there is an option in Win10 Sound Settings to "enable"
> > > > > UAC2 by default. I cannot find that option and I wonder if this is
> > > > > what is preventing me from changing the sample rate or channel mask?
> > > > > Could Windows be treating my audio gadget as a UAC1 device rather than
> > > > > a fully multi-channel audio device (even though the usbaudio2.sys
> > > > > driver is loaded)? Have you tried other configurations to verify your
> > > > > Win10 box supports more than two channels and a 44.1K sample rate? I
> > > > > look forward to your feedback and any suggestions you might offer.
> > > > >
> > > >
> > > > I was able to reproduce the issue and prepared a patch below, please
> > > > try it and see if it fixes the issue.
> > > >
> > > > Maximum data rates that I used were (AFAIR) 8 channel 192kHz/32bit,
> > > > but there is another issue with high data rate if someone uses so many
> > > > channels, very high sampling frequency or sample size that data can't
> > > > fit into allowed (by USB spec) max packet size of endpoint. In this case
> > > > need to decrease bInterval of endpoint.
> > > > I test it in a high-performance audio case so always use a minimal possible
> > > > bInterval (set to '1').
> > > > Of course in the future that need to be calculated dynamically depending on
> > > > the UAC2 settings
> > > >
> > > > Please test patches below and see it it helps
> > > >
> > > > ---------------------------------------------8<----------------------------------------
> > > > From 51516435bbf2486574ec7bc9fd4726677cd474a4 Mon Sep 17 00:00:00 2001
> > > > From: Ruslan Bilovol <ruslan.bilovol@gmail.com>
> > > > Date: Sun, 22 Nov 2020 21:05:38 +0200
> > > > Subject: [PATCH] usb: gadget: f_uac2: always increase endpoint max_packet_size
> > > > by one audio slot
> > > >
> > > > As per UAC2 Audio Data Formats spec (2.3.1.1 USB Packets),
> > > > if the sampling rate is a constant, the allowable variation
> > > > of number of audio slots per virtual frame is +/- 1 audio slot.
> > > >
> > > > It means that endpoint should be able to accept/send +1 audio
> > > > slot.
> > > >
> > > > Previous endpoint max_packet_size calculation code
> > > > was adding sometimes +1 audio slot due to DIV_ROUND_UP
> > > > behaviour which was rounding up to closest integer.
> > > > However this doesn't work if the numbers are divisible.
> > > >
> > > > It had no any impact with Linux hosts which ignore
> > > > this issue, but in case of more strict Windows it
> > > > caused rejected enumeration
> > > >
> > > > Thus always add +1 audio slot to endpoint's max packet size
> > > >
> > > > Signed-off-by: Ruslan Bilovol <ruslan.bilovol@gmail.com>
> > > > ---
> > > > drivers/usb/gadget/function/f_uac2.c | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/usb/gadget/function/f_uac2.c
> > > > b/drivers/usb/gadget/function/f_uac2.c
> > > > index ebdb2a1..1698587 100644
> > > > --- a/drivers/usb/gadget/function/f_uac2.c
> > > > +++ b/drivers/usb/gadget/function/f_uac2.c
> > > > @@ -486,7 +486,7 @@ static void set_ep_max_packet_size(const struct
> > > > f_uac2_opts *uac2_opts,
> > > > }
> > > >
> > > > max_packet_size = num_channels(chmask) * ssize *
> > > > - DIV_ROUND_UP(srate, factor / (1 << (ep_desc->bInterval - 1)));
> > > > + ((srate / (factor / (1 << (ep_desc->bInterval - 1)))) + 1);
> > > >
> > > > if (!is_playback && (uac2_opts->c_sync == USB_ENDPOINT_SYNC_ASYNC))
> > > > max_packet_size = max_packet_size * FBACK_FREQ_MAX / 100;
> > > > --
> > > > 1.9.1
> > > >
> > > >
> > > > ---------------------------------------------8<----------------------------------------
> > > > From c8f2f2b414af672ec40841e75fb1ea761ae29122 Mon Sep 17 00:00:00 2001
> > > > From: Ruslan Bilovol <ruslan.bilovol@gmail.com>
> > > > Date: Sun, 16 Feb 2020 22:40:23 +0200
> > > > Subject: [PATCH] usb: gadget: f_uac2: change bInterval to 1 for
> > > > 8ch/24bit/44.1kHz
> > > >
> > > > With bInterval=4 one audio slot in case of 8ch/24bit/44.1kHz
> > > > exeeds MaxPacket size of ISO endpoint (>1024) and can't be
> > > > transferred in time.
> > > > USB spec ("5.6.3 Isochronous Transfer Packet Size Constraints")
> > > > says if we need to transfer more than 1024 it is a high speed, high
> > > > bandwidth endpoint and should have bInterval=1.
> > > >
> > > > In the future, bInterval should be dynamically calculated
> > > > depending on bandwith requirementes
> > > >
> > > > Signed-off-by: Ruslan Bilovol <ruslan.bilovol@gmail.com>
> > > > ---
> > > > drivers/usb/gadget/function/f_uac2.c | 4 ++--
> > > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/usb/gadget/function/f_uac2.c
> > > > b/drivers/usb/gadget/function/f_uac2.c
> > > > index 3633df6..fb9b875 100644
> > > > --- a/drivers/usb/gadget/function/f_uac2.c
> > > > +++ b/drivers/usb/gadget/function/f_uac2.c
> > > > @@ -281,7 +281,7 @@ enum {
> > > >
> > > > .bmAttributes = USB_ENDPOINT_XFER_ISOC | USB_ENDPOINT_SYNC_ASYNC,
> > > > .wMaxPacketSize = cpu_to_le16(1024),
> > > > - .bInterval = 4,
> > > > + .bInterval = 1,
> > > > };
> > > >
> > > > /* CS AS ISO OUT Endpoint */
> > > > @@ -358,7 +358,7 @@ enum {
> > > >
> > > > .bmAttributes = USB_ENDPOINT_XFER_ISOC | USB_ENDPOINT_SYNC_ASYNC,
> > > > .wMaxPacketSize = cpu_to_le16(1024),
> > > > - .bInterval = 4,
> > > > + .bInterval = 1,
> > > > };
> > > >
> > > > /* CS AS ISO IN Endpoint */
> > > > --
> > > > 1.9.1
> > > >
> > > >
> > > > ---------------------------------------------8<----------------------------------------
> > > >
> > > > Thanks,
> > > > Ruslan
> > >
> > > Hi Ruslan -
> > >
> > > I have had some mixed success with your additional patches. First, the
> >
> > That's good news, thanks for testing!
> >
> > > UAC2 gadget does correctly register with Windows 10 but only for 1-31
> > > channels. For a fully specified channel map (32 channels):
> > >
> > > c_chmask = 0xFFFFFFFF ( Does not load)
> > > p_chmask = 0xFFFFFFFF ( Does not load)
> > >
> > > the driver fails to load (code=10). This seems similar to the prior
> > > problem. Could there be another off-by-one problem here?
> > >
> > > With a 31 channel mask the Windows 10 usbaudio2.sys loads successfully, e.g.
> > >
> > > c_chmask = 0x7FFFFFFF (loads)
> > > p_chmask = 0x7FFFFFFF (loads)
> >
> > That's correct behavior. UAC2 spec says that bits D27..D30 are reserved.
> > Also bit D31 is mutually exclusive with other bits and means 'raw data'.
> > It means that with current UAC2 gadget implementation you can enable
> > only 27 channels.
> >
> > It seems like it's possible to add more channels by adding more audio
> > clusters but that requires UAC2 driver modifications. Also it's not clear
> > whether it's implemented in Host OS drivers
> >
> > I personally tested as many as 8x8 playback/capture channels simultaneously.
> >
> > >
> > > Once Windows 10 loads the driver successfully, however, I can't seem
> > > to select the device through Audacity on Windows. The device appears
> > > in the Windows "Device Manager" under "Sound, video and game
> > > controllers -> Source/Sink". How are you testing audio
> > > playback/capture under Windows? Perhaps my test approach is wrong?
> >
> > You need to go to sound settings -> manage sound devices and enable
> > UAC2 gadget there, after that it will appear in the system.
> >
> > I also used Audacity for testing this and it works fine for me
> >
> > Thanks,
> > Ruslan
> >
> > >
> > > I used the same approach on my Linux host where the USB UAC2 device
> > > appears as an ALSA device and I use speaker-test on the target
> > > (BeagleBone Black) to play and then capture the audio using Audacity
> > > on the Linux host. I was hoping to do the same or something similar
> > > under Windows.
> > >
> > > Below is the script I've been using on the target to configure my UAC2 device:
> > >
> > > mkdir -p "/sys/kernel/config/usb_gadget/g_multi"
> > > echo "${idVendor:-0x11F5}" > "/sys/kernel/config/usb_gadget/g_multi/idVendor"
> > > echo "${idProduct:-0x0009}" > "/sys/kernel/config/usb_gadget/g_multi/idProduct"
> > > echo "${bcdDevice:-0x0001}" > "/sys/kernel/config/usb_gadget/g_multi/bcdDevice"
> > > echo "${bDeviceClass:-0x00}" >
> > > "/sys/kernel/config/usb_gadget/g_multi/bDeviceClass"
> > > echo "${bDeviceSubClass:-0x00}" >
> > > "/sys/kernel/config/usb_gadget/g_multi/bDeviceSubClass"
> > > echo "${bDeviceProtocol:-0x00}" >
> > > "/sys/kernel/config/usb_gadget/g_multi/bDeviceProtocol"
> > > echo "${bcdUSB:-0x0200}" > "/sys/kernel/config/usb_gadget/g_multi/bcdUSB"
> > > echo "${bMaxPacketSize0:-0x00}" >
> > > "/sys/kernel/config/usb_gadget/g_multi/bMaxPacketSize0"
> > > mkdir -p "/sys/kernel/config/usb_gadget/g_multi/strings/0x409"
> > > echo "${serialnumber:-000000000000}" >
> > > "/sys/kernel/config/usb_gadget/g_multi/strings/0x409/serialnumber"
> > > echo "${product:-UAC2}" >
> > > "/sys/kernel/config/usb_gadget/g_multi/strings/0x409/product"
> > > echo "${manufacturer:-www.acme.com}" >
> > > "/sys/kernel/config/usb_gadget/g_multi/strings/0x409/manufacturer"
> > > mkdir -p "/sys/kernel/config/usb_gadget/g_multi/functions/uac2.usb0"
> > > echo "${req_number:-4}" >
> > > "/sys/kernel/config/usb_gadget/g_multi/functions/uac2.usb0/req_number"
> > > echo "${c_ssize:-4}" >
> > > "/sys/kernel/config/usb_gadget/g_multi/functions/uac2.usb0/c_ssize"
> > > echo "${c_srate:-48000}" >
> > > "/sys/kernel/config/usb_gadget/g_multi/functions/uac2.usb0/c_srate"
> > > echo "${c_chmask:-0x00000003}" >
> > > "/sys/kernel/config/usb_gadget/g_multi/functions/uac2.usb0/c_chmask"
> > > echo "${c_sync:-asynchronous}" >
> > > "/sys/kernel/config/usb_gadget/g_multi/functions/uac2.usb0/c_sync"
> > > echo "${p_ssize:-4}" >
> > > "/sys/kernel/config/usb_gadget/g_multi/functions/uac2.usb0/p_ssize"
> > > echo "${p_srate:-48000}" >
> > > "/sys/kernel/config/usb_gadget/g_multi/functions/uac2.usb0/p_srate"
> > > echo "${p_chmask:-0x00000003}" >
> > > "/sys/kernel/config/usb_gadget/g_multi/functions/uac2.usb0/p_chmask"
> > > mkdir -p "/sys/kernel/config/usb_gadget/g_multi/configs/c.1"
> > > echo "${bmAttributes:-0x80}" >
> > > "/sys/kernel/config/usb_gadget/g_multi/configs/c.1/bmAttributes"
> > > echo "${MaxPower:-500}" >
> > > "/sys/kernel/config/usb_gadget/g_multi/configs/c.1/MaxPower"
> > > mkdir -p "/sys/kernel/config/usb_gadget/g_multi/configs/c.1/strings/0x409"
> > > echo "${configuration:-BeagleBone Composite}" >
> > > "/sys/kernel/config/usb_gadget/g_multi/configs/c.1/strings/0x409/configuration"
> > > ln -s "/sys/kernel/config/usb_gadget/g_multi/functions/uac2.usb0"
> > > "/sys/kernel/config/usb_gadget/g_multi/configs/c.1/uac2.usb0"
> > > mkdir -p "/sys/kernel/config/usb_gadget/g_multi/os_desc"
> > > ln -s "//sys/kernel/config/usb_gadget/g_multi/configs/c.1"
> > > "/sys/kernel/config/usb_gadget/g_multi/os_desc/c.1"
> > > echo "${qw_sign:-MSFT100}" >
> > > "/sys/kernel/config/usb_gadget/g_multi/os_desc/qw_sign"
> > > echo "${b_vendor_code:-0x00}" >
> > > "/sys/kernel/config/usb_gadget/g_multi/os_desc/b_vendor_code"
> > > echo "${use:-1}" > "/sys/kernel/config/usb_gadget/g_multi/os_desc/use"
> > >
> > > basename /sys/class/udc/* > /sys/kernel/config/usb_gadget/g_multi/UDC
> > >
> > > This is only a two channel playback/capture configuration. I hoped to
> > > get this working under Windows before increasing my channel count to a
> > > full 32 channel mask with a 48K sample rate.
> > >
> > > I look forward to hearing your suggestions and possibly pointing out
> > > where I've gone astray.
> > >
> > > Thanks,
> > >
> > > Glenn
>
> Hi Ruslan -
>
> Thanks for the feedback but unfortunately I've experienced mixed
> results with the gadget UAC2 driver on both Windows/Linux. Let me
> describe my environment. My host platform is either a Linux Ubuntu
> 18.04 or Windows 10 laptop while the target environment is a
> BeagleBone Black (Linux beaglebone 5.4.74-g9574bba32a #1 PREEMPT). I'm
> testing two different scenarios:
>
> Scenario #1:
> BeagleBone Black (BBB) runs speaker-test generating a single channel
> (S32_LE) audio stream containing a 1KHz tone with a 48K sample rate,
> e.g.
>
> > speaker-test -D hw:1,0 -r 48000 -c 1 -f 1000 -F S32_LE -t sine
>
> The host laptop is running Audacity and recording the tone over the
> UAC2 adapter. On the Linux host the capture is correct and the tone is
> bit-perfect. On the Windows 10 the capture contains numerous missing
> samples which translates into a lot of audible pops and clicks.
>
> Scenario #2:
> The Linux/Windows host plays a single channel, 48K, S32_LE 1K sine
> tone to the target using either Audacity (on Windows) or 'aplay' (on
> Linux), e.g.
>
> > aplay -D hw:4,0 -c 1 -r 48000 -t wav tone_1k.wav (Linux)
>
> On the BBB target I use 'arecord' to record the tone to a RAM disk and
> then copy the recorded file back to the host where I can verify the
> quality of the recording. In both instances (e.g. using either Windows
> or Linux for playback) the recording on the target results in a
> captured file with missing samples and audible pops/clicks. In this
> scenario the UAC2 gadget is configured with c_sync == asynchronous. I
> wouldn't expect things to improve with c_sync == adaptive since you
> mentioned in your patch that it always reports back the nominal
> frequency to the host from the feedback endpoint.
>
> Do you have any suggestions that might explain (the above) behavior.
> Can you describe your test environment in more detail so that I can
> perhaps re-create it? What Linux target are you using with your tests?
> You mentioned you tested an 8x8 playback/capture scenario. Can you
> provide any details of how you performed this test and the method you
> used to confirm the audio quality for the capture/playback?
>
> Thanks for any insights you might be able to offer . . .
>
> Glenn
Hi Ruslan -
This is a follow-up from my post yesterday. I recompiled my kernel
*WITHOUT* your UAC2 patches and repeated Scenario #2 where the Linux
PC plays a single channel tone to the BeagleBone Black where it's
recorded with 'arecord'. Yesterday, I recorded garbled audio on the
target but today, without any UAC2 kernel patches, the recorded audio
on the target is glitch-free and appears to be bit-perfect.
This experiment leads me to believe your patches may be inadvertently
corrupting the data-path. Have you been able to repeat my experiment
and either confirm or refute my findings? I am interested to learn
more how you tested your patches and whether it's something I can
recreate here.
Assuming we can sort out this data corruption issue, what are your
thoughts on how the Linux target device can properly provide the
Windows feedback endpoint with real frequency updates rather than the
constant nominal frequency. If I understood your patch notes correctly
it seems this is an outstanding issue that requires additional
attention. I'm a bit of a noob when it comes to how this might be
addressed.
Thanks for your continued insights and support . . .
Glenn
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 0/3] UAC2 Gadget: feedback endpoint support
2020-12-02 22:04 ` Glenn Schmottlach
@ 2020-12-03 10:09 ` Peter Chen
2020-12-03 22:07 ` Glenn Schmottlach
2020-12-10 12:59 ` Ruslan Bilovol
0 siblings, 2 replies; 31+ messages in thread
From: Peter Chen @ 2020-12-03 10:09 UTC (permalink / raw)
To: Glenn Schmottlach; +Cc: Ruslan Bilovol, balbi, linux-usb
On 20-12-02 17:04:47, Glenn Schmottlach wrote:
> On Tue, Dec 1, 2020 at 4:43 PM Glenn Schmottlach <gschmottlach@gmail.com> wrote:
> > Hi Ruslan -
> >
> > Thanks for the feedback but unfortunately I've experienced mixed
> > results with the gadget UAC2 driver on both Windows/Linux. Let me
> > describe my environment. My host platform is either a Linux Ubuntu
> > 18.04 or Windows 10 laptop while the target environment is a
> > BeagleBone Black (Linux beaglebone 5.4.74-g9574bba32a #1 PREEMPT). I'm
> > testing two different scenarios:
> >
> > Scenario #1:
> > BeagleBone Black (BBB) runs speaker-test generating a single channel
> > (S32_LE) audio stream containing a 1KHz tone with a 48K sample rate,
> > e.g.
> >
> > > speaker-test -D hw:1,0 -r 48000 -c 1 -f 1000 -F S32_LE -t sine
> >
> > The host laptop is running Audacity and recording the tone over the
> > UAC2 adapter. On the Linux host the capture is correct and the tone is
> > bit-perfect. On the Windows 10 the capture contains numerous missing
> > samples which translates into a lot of audible pops and clicks.
> >
> > Scenario #2:
> > The Linux/Windows host plays a single channel, 48K, S32_LE 1K sine
> > tone to the target using either Audacity (on Windows) or 'aplay' (on
> > Linux), e.g.
> >
> > > aplay -D hw:4,0 -c 1 -r 48000 -t wav tone_1k.wav (Linux)
> >
> > On the BBB target I use 'arecord' to record the tone to a RAM disk and
> > then copy the recorded file back to the host where I can verify the
> > quality of the recording. In both instances (e.g. using either Windows
> > or Linux for playback) the recording on the target results in a
> > captured file with missing samples and audible pops/clicks. In this
> > scenario the UAC2 gadget is configured with c_sync == asynchronous. I
> > wouldn't expect things to improve with c_sync == adaptive since you
> > mentioned in your patch that it always reports back the nominal
> > frequency to the host from the feedback endpoint.
> >
> > Do you have any suggestions that might explain (the above) behavior.
> > Can you describe your test environment in more detail so that I can
> > perhaps re-create it? What Linux target are you using with your tests?
> > You mentioned you tested an 8x8 playback/capture scenario. Can you
> > provide any details of how you performed this test and the method you
> > used to confirm the audio quality for the capture/playback?
> >
> > Thanks for any insights you might be able to offer . . .
> >
> > Glenn
>
> Hi Ruslan -
>
> This is a follow-up from my post yesterday. I recompiled my kernel
> *WITHOUT* your UAC2 patches and repeated Scenario #2 where the Linux
> PC plays a single channel tone to the BeagleBone Black where it's
> recorded with 'arecord'. Yesterday, I recorded garbled audio on the
> target but today, without any UAC2 kernel patches, the recorded audio
> on the target is glitch-free and appears to be bit-perfect.
>
> This experiment leads me to believe your patches may be inadvertently
> corrupting the data-path. Have you been able to repeat my experiment
> and either confirm or refute my findings? I am interested to learn
> more how you tested your patches and whether it's something I can
> recreate here.
>
> Assuming we can sort out this data corruption issue, what are your
> thoughts on how the Linux target device can properly provide the
> Windows feedback endpoint with real frequency updates rather than the
> constant nominal frequency. If I understood your patch notes correctly
> it seems this is an outstanding issue that requires additional
> attention. I'm a bit of a noob when it comes to how this might be
> addressed.
>
> Thanks for your continued insights and support . . .
>
> Glenn
Hi Glenn & Ruslan,
Do you know why WIN10 can't recognized UAC2 device if I configure the
sample rate as 48000HZ? Configuring sample rate as 44100HZ, the playback
function would work well at my platforms (chipidea IP), no glitch is
heard. At WIN10, I use Windows Media Player, at board side I use command:
arecord -f cd -t wav -D hw:4,0 | aplay -f cd -Dplughw:3,0 &
From the USB Bus analyzer:
Feedback EP is scheduled every 1ms, there are nine 176-byte packets and one
180-byte packet among 10ms transfers.
--
Thanks,
Peter Chen
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 0/3] UAC2 Gadget: feedback endpoint support
2020-12-03 10:09 ` Peter Chen
@ 2020-12-03 22:07 ` Glenn Schmottlach
2020-12-10 12:59 ` Ruslan Bilovol
1 sibling, 0 replies; 31+ messages in thread
From: Glenn Schmottlach @ 2020-12-03 22:07 UTC (permalink / raw)
To: Peter Chen; +Cc: Ruslan Bilovol, balbi, linux-usb
On Thu, Dec 3, 2020 at 5:09 AM Peter Chen <peter.chen@nxp.com> wrote:
>
> On 20-12-02 17:04:47, Glenn Schmottlach wrote:
> > On Tue, Dec 1, 2020 at 4:43 PM Glenn Schmottlach <gschmottlach@gmail.com> wrote:
> > > Hi Ruslan -
> > >
> > > Thanks for the feedback but unfortunately I've experienced mixed
> > > results with the gadget UAC2 driver on both Windows/Linux. Let me
> > > describe my environment. My host platform is either a Linux Ubuntu
> > > 18.04 or Windows 10 laptop while the target environment is a
> > > BeagleBone Black (Linux beaglebone 5.4.74-g9574bba32a #1 PREEMPT). I'm
> > > testing two different scenarios:
> > >
> > > Scenario #1:
> > > BeagleBone Black (BBB) runs speaker-test generating a single channel
> > > (S32_LE) audio stream containing a 1KHz tone with a 48K sample rate,
> > > e.g.
> > >
> > > > speaker-test -D hw:1,0 -r 48000 -c 1 -f 1000 -F S32_LE -t sine
> > >
> > > The host laptop is running Audacity and recording the tone over the
> > > UAC2 adapter. On the Linux host the capture is correct and the tone is
> > > bit-perfect. On the Windows 10 the capture contains numerous missing
> > > samples which translates into a lot of audible pops and clicks.
> > >
> > > Scenario #2:
> > > The Linux/Windows host plays a single channel, 48K, S32_LE 1K sine
> > > tone to the target using either Audacity (on Windows) or 'aplay' (on
> > > Linux), e.g.
> > >
> > > > aplay -D hw:4,0 -c 1 -r 48000 -t wav tone_1k.wav (Linux)
> > >
> > > On the BBB target I use 'arecord' to record the tone to a RAM disk and
> > > then copy the recorded file back to the host where I can verify the
> > > quality of the recording. In both instances (e.g. using either Windows
> > > or Linux for playback) the recording on the target results in a
> > > captured file with missing samples and audible pops/clicks. In this
> > > scenario the UAC2 gadget is configured with c_sync == asynchronous. I
> > > wouldn't expect things to improve with c_sync == adaptive since you
> > > mentioned in your patch that it always reports back the nominal
> > > frequency to the host from the feedback endpoint.
> > >
> > > Do you have any suggestions that might explain (the above) behavior.
> > > Can you describe your test environment in more detail so that I can
> > > perhaps re-create it? What Linux target are you using with your tests?
> > > You mentioned you tested an 8x8 playback/capture scenario. Can you
> > > provide any details of how you performed this test and the method you
> > > used to confirm the audio quality for the capture/playback?
> > >
> > > Thanks for any insights you might be able to offer . . .
> > >
> > > Glenn
> >
> > Hi Ruslan -
> >
> > This is a follow-up from my post yesterday. I recompiled my kernel
> > *WITHOUT* your UAC2 patches and repeated Scenario #2 where the Linux
> > PC plays a single channel tone to the BeagleBone Black where it's
> > recorded with 'arecord'. Yesterday, I recorded garbled audio on the
> > target but today, without any UAC2 kernel patches, the recorded audio
> > on the target is glitch-free and appears to be bit-perfect.
> >
> > This experiment leads me to believe your patches may be inadvertently
> > corrupting the data-path. Have you been able to repeat my experiment
> > and either confirm or refute my findings? I am interested to learn
> > more how you tested your patches and whether it's something I can
> > recreate here.
> >
> > Assuming we can sort out this data corruption issue, what are your
> > thoughts on how the Linux target device can properly provide the
> > Windows feedback endpoint with real frequency updates rather than the
> > constant nominal frequency. If I understood your patch notes correctly
> > it seems this is an outstanding issue that requires additional
> > attention. I'm a bit of a noob when it comes to how this might be
> > addressed.
> >
> > Thanks for your continued insights and support . . .
> >
> > Glenn
>
> Hi Glenn & Ruslan,
>
> Do you know why WIN10 can't recognized UAC2 device if I configure the
> sample rate as 48000HZ? Configuring sample rate as 44100HZ, the playback
> function would work well at my platforms (chipidea IP), no glitch is
> heard. At WIN10, I use Windows Media Player, at board side I use command:
>
> arecord -f cd -t wav -D hw:4,0 | aplay -f cd -Dplughw:3,0 &
>
> From the USB Bus analyzer:
>
> Feedback EP is scheduled every 1ms, there are nine 176-byte packets and one
> 180-byte packet among 10ms transfers.
>
> --
>
> Thanks,
> Peter Chen
Hi Peter -
I have not experienced the issue you are reporting. My configuration
uses a 48Khz sample rate exclusively and Windows 10 appears to
recognize the UAC2 device properly with all of Ruslan's patches. I
have not done any serious testing at the 44.1KHz rate so I don't
recall if that is recognized correctly on my Windows 10 box.
It looks like you've established a UAC2 loopback on your target board.
How are you verifying the captured audio on the Window 10 PC? I didn't
realize Windows Media Player allows you to both playback and capture
from the same (UAC2) device simultaneously.
Thanks,
Glenn
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 2/3] usb: gadget: f_uac2: add adaptive sync support for capture
2020-11-26 11:13 ` Jerome Brunet
@ 2020-12-04 14:03 ` Ruslan Bilovol
0 siblings, 0 replies; 31+ messages in thread
From: Ruslan Bilovol @ 2020-12-04 14:03 UTC (permalink / raw)
To: Jerome Brunet; +Cc: Felipe Balbi, Linux USB, Glenn Schmottlach
On Thu, Nov 26, 2020 at 1:13 PM Jerome Brunet <jbrunet@baylibre.com> wrote:
>
>
> On Sun 08 Nov 2020 at 01:18, Ruslan Bilovol <ruslan.bilovol@gmail.com> wrote:
>
> > Current f_uac2 USB OUT (aka 'capture') synchronization
> > implements 'ASYNC' scenario which means USB Gadget has
> > it's own freerunning clock and can update Host about
> > real clock frequency through feedback endpoint so Host
> > can align number of samples sent to the USB gadget to
> > prevent overruns/underruns
> >
> > In case if Gadget can has no it's internal clock and
> > can consume audio samples at any rate (for example,
> > on the Gadget side someone records audio directly to
> > a file, or audio samples are played through an
> > external DAC as soon as they arrive), UAC2 spec
> > suggests 'ADAPTIVE' synchronization type.
> >
> > Change UAC2 driver to make it configurable through
> > additional 'c_sync' configfs file.
> >
> > Default remains 'asynchronous' with possibility to
> > switch it to 'adaptive'
> >
> > Signed-off-by: Ruslan Bilovol <ruslan.bilovol@gmail.com>
> > ---
> > Documentation/ABI/testing/configfs-usb-gadget-uac2 | 1 +
> > Documentation/usb/gadget-testing.rst | 1 +
> > drivers/usb/gadget/function/f_uac2.c | 96 ++++++++++++++++++++--
> > drivers/usb/gadget/function/u_uac2.h | 2 +
> > 4 files changed, 91 insertions(+), 9 deletions(-)
> >
> > diff --git a/Documentation/ABI/testing/configfs-usb-gadget-uac2 b/Documentation/ABI/testing/configfs-usb-gadget-uac2
> > index 2bfdd4e..4fbff96 100644
> > --- a/Documentation/ABI/testing/configfs-usb-gadget-uac2
> > +++ b/Documentation/ABI/testing/configfs-usb-gadget-uac2
> > @@ -7,6 +7,7 @@ Description:
> > c_chmask - capture channel mask
> > c_srate - capture sampling rate
> > c_ssize - capture sample size (bytes)
> > + c_sync - capture synchronization type (async/adaptive)
> > p_chmask - playback channel mask
> > p_srate - playback sampling rate
> > p_ssize - playback sample size (bytes)
> > diff --git a/Documentation/usb/gadget-testing.rst b/Documentation/usb/gadget-testing.rst
> > index 2eeb3e9..360a7ca 100644
> > --- a/Documentation/usb/gadget-testing.rst
> > +++ b/Documentation/usb/gadget-testing.rst
> > @@ -728,6 +728,7 @@ The uac2 function provides these attributes in its function directory:
> > c_chmask capture channel mask
> > c_srate capture sampling rate
> > c_ssize capture sample size (bytes)
> > + c_sync capture synchronization type (async/adaptive)
> > p_chmask playback channel mask
> > p_srate playback sampling rate
> > p_ssize playback sample size (bytes)
> > diff --git a/drivers/usb/gadget/function/f_uac2.c b/drivers/usb/gadget/function/f_uac2.c
> > index a57bf77..3187ad3 100644
> > --- a/drivers/usb/gadget/function/f_uac2.c
> > +++ b/drivers/usb/gadget/function/f_uac2.c
> > @@ -41,6 +41,7 @@
> >
> > #define EPIN_EN(_opts) ((_opts)->p_chmask != 0)
> > #define EPOUT_EN(_opts) ((_opts)->c_chmask != 0)
> > +#define EPOUT_FBACK_IN_EN(_opts) ((_opts)->c_sync == USB_ENDPOINT_SYNC_ASYNC)
> >
> > struct f_uac2 {
> > struct g_audio g_audio;
> > @@ -237,7 +238,7 @@ enum {
> > .bDescriptorType = USB_DT_INTERFACE,
> >
> > .bAlternateSetting = 1,
> > - .bNumEndpoints = 2,
> > + .bNumEndpoints = 1,
> > .bInterfaceClass = USB_CLASS_AUDIO,
> > .bInterfaceSubClass = USB_SUBCLASS_AUDIOSTREAMING,
> > .bInterfaceProtocol = UAC_VERSION_2,
> > @@ -270,7 +271,7 @@ enum {
> > .bDescriptorType = USB_DT_ENDPOINT,
> >
> > .bEndpointAddress = USB_DIR_OUT,
> > - .bmAttributes = USB_ENDPOINT_XFER_ISOC | USB_ENDPOINT_SYNC_ASYNC,
> > + .bmAttributes = USB_ENDPOINT_XFER_ISOC,
> > .wMaxPacketSize = cpu_to_le16(1023),
> > .bInterval = 1,
> > };
> > @@ -279,7 +280,7 @@ enum {
> > .bLength = USB_DT_ENDPOINT_SIZE,
> > .bDescriptorType = USB_DT_ENDPOINT,
> >
> > - .bmAttributes = USB_ENDPOINT_XFER_ISOC | USB_ENDPOINT_SYNC_ASYNC,
> > + .bmAttributes = USB_ENDPOINT_XFER_ISOC,
> > .wMaxPacketSize = cpu_to_le16(1024),
> > .bInterval = 1,
> > };
> > @@ -540,6 +541,19 @@ static void setup_descriptor(struct f_uac2_opts *opts)
> > len += sizeof(io_out_ot_desc);
> > ac_hdr_desc.wTotalLength = cpu_to_le16(len);
> > iad_desc.bInterfaceCount++;
> > +
> > + if (EPOUT_FBACK_IN_EN(opts)) {
> > + fs_epout_desc.bmAttributes |=
> > + USB_ENDPOINT_SYNC_ASYNC;
> > + hs_epout_desc.bmAttributes |=
> > + USB_ENDPOINT_SYNC_ASYNC;
> > + std_as_out_if1_desc.bNumEndpoints++;
> > + } else {
> > + fs_epout_desc.bmAttributes |=
> > + USB_ENDPOINT_SYNC_ADAPTIVE;
> > + hs_epout_desc.bmAttributes |=
> > + USB_ENDPOINT_SYNC_ADAPTIVE;
> > + }
>
> Hi Ruslan,
>
> First, thanks a lot for this series, it is very helpful
>
> Instead of amending the descriptors, could you consider using comments like
>
> /* .bNumEndpoints = DYNAMIC */
>
> or
>
> /* .bmAttributes = DYNAMIC */
Agree, I just forgot to do that
>
> It would help understand that the actual value of these parameters will be
> determine at runtime, making the code easier to follow and maintain.
>
> Also, I wonder if it would be difficult to add implicit feedback support
> while at it ?
>
> I'm asking this now because it could (possibly) change the driver params
> (implicit is async as well)
It seems it should be quite easy to add it, so at least worth a try.
Will experiment this in next patch set
Thanks,
Ruslan
>
> > }
> >
> > i = 0;
> > @@ -564,7 +578,8 @@ static void setup_descriptor(struct f_uac2_opts *opts)
> > fs_audio_desc[i++] = USBDHDR(&as_out_fmt1_desc);
> > fs_audio_desc[i++] = USBDHDR(&fs_epout_desc);
> > fs_audio_desc[i++] = USBDHDR(&as_iso_out_desc);
> > - fs_audio_desc[i++] = USBDHDR(&fs_epin_fback_desc);
> > + if (EPOUT_FBACK_IN_EN(opts))
> > + fs_audio_desc[i++] = USBDHDR(&fs_epin_fback_desc);
> > }
> > if (EPIN_EN(opts)) {
> > fs_audio_desc[i++] = USBDHDR(&std_as_in_if0_desc);
> > @@ -598,7 +613,8 @@ static void setup_descriptor(struct f_uac2_opts *opts)
> > hs_audio_desc[i++] = USBDHDR(&as_out_fmt1_desc);
> > hs_audio_desc[i++] = USBDHDR(&hs_epout_desc);
> > hs_audio_desc[i++] = USBDHDR(&as_iso_out_desc);
> > - hs_audio_desc[i++] = USBDHDR(&hs_epin_fback_desc);
> > + if (EPOUT_FBACK_IN_EN(opts))
> > + hs_audio_desc[i++] = USBDHDR(&hs_epin_fback_desc);
> > }
> > if (EPIN_EN(opts)) {
> > hs_audio_desc[i++] = USBDHDR(&std_as_in_if0_desc);
> > @@ -706,11 +722,14 @@ static void setup_descriptor(struct f_uac2_opts *opts)
> > dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
> > return -ENODEV;
> > }
> > - agdev->in_ep_fback = usb_ep_autoconfig(gadget,
> > + if (EPOUT_FBACK_IN_EN(uac2_opts)) {
> > + agdev->in_ep_fback = usb_ep_autoconfig(gadget,
> > &fs_epin_fback_desc);
> > - if (!agdev->in_ep_fback) {
> > - dev_err(dev, "%s:%d Error!\n", __func__, __LINE__);
> > - return -ENODEV;
> > + if (!agdev->in_ep_fback) {
> > + dev_err(dev, "%s:%d Error!\n",
> > + __func__, __LINE__);
> > + return -ENODEV;
> > + }
> > }
> > }
> >
> > @@ -1057,11 +1076,68 @@ static void f_uac2_attr_release(struct config_item *item)
> > \
> > CONFIGFS_ATTR(f_uac2_opts_, name)
> >
> > +#define UAC2_ATTRIBUTE_SYNC(name) \
> > +static ssize_t f_uac2_opts_##name##_show(struct config_item *item, \
> > + char *page) \
> > +{ \
> > + struct f_uac2_opts *opts = to_f_uac2_opts(item); \
> > + int result; \
> > + char *str; \
> > + \
> > + mutex_lock(&opts->lock); \
> > + switch (opts->name) { \
> > + case USB_ENDPOINT_SYNC_ASYNC: \
> > + str = "async"; \
> > + break; \
> > + case USB_ENDPOINT_SYNC_ADAPTIVE: \
> > + str = "adaptive"; \
> > + break; \
> > + default: \
> > + str = "unknown"; \
> > + break; \
> > + } \
> > + result = sprintf(page, "%s\n", str); \
> > + mutex_unlock(&opts->lock); \
> > + \
> > + return result; \
> > +} \
> > + \
> > +static ssize_t f_uac2_opts_##name##_store(struct config_item *item, \
> > + const char *page, size_t len) \
> > +{ \
> > + struct f_uac2_opts *opts = to_f_uac2_opts(item); \
> > + int ret = 0; \
> > + \
> > + mutex_lock(&opts->lock); \
> > + if (opts->refcnt) { \
> > + ret = -EBUSY; \
> > + goto end; \
> > + } \
> > + \
> > + if (!strncmp(page, "async", 5)) \
> > + opts->name = USB_ENDPOINT_SYNC_ASYNC; \
> > + else if (!strncmp(page, "adaptive", 8)) \
> > + opts->name = USB_ENDPOINT_SYNC_ADAPTIVE; \
> > + else { \
> > + ret = -EINVAL; \
> > + goto end; \
> > + } \
> > + \
> > + ret = len; \
> > + \
> > +end: \
> > + mutex_unlock(&opts->lock); \
> > + return ret; \
> > +} \
> > + \
> > +CONFIGFS_ATTR(f_uac2_opts_, name)
> > +
> > UAC2_ATTRIBUTE(p_chmask);
> > UAC2_ATTRIBUTE(p_srate);
> > UAC2_ATTRIBUTE(p_ssize);
> > UAC2_ATTRIBUTE(c_chmask);
> > UAC2_ATTRIBUTE(c_srate);
> > +UAC2_ATTRIBUTE_SYNC(c_sync);
> > UAC2_ATTRIBUTE(c_ssize);
> > UAC2_ATTRIBUTE(req_number);
> >
> > @@ -1072,6 +1148,7 @@ static void f_uac2_attr_release(struct config_item *item)
> > &f_uac2_opts_attr_c_chmask,
> > &f_uac2_opts_attr_c_srate,
> > &f_uac2_opts_attr_c_ssize,
> > + &f_uac2_opts_attr_c_sync,
> > &f_uac2_opts_attr_req_number,
> > NULL,
> > };
> > @@ -1110,6 +1187,7 @@ static struct usb_function_instance *afunc_alloc_inst(void)
> > opts->c_chmask = UAC2_DEF_CCHMASK;
> > opts->c_srate = UAC2_DEF_CSRATE;
> > opts->c_ssize = UAC2_DEF_CSSIZE;
> > + opts->c_sync = UAC2_DEF_CSYNC;
> > opts->req_number = UAC2_DEF_REQ_NUM;
> > return &opts->func_inst;
> > }
> > diff --git a/drivers/usb/gadget/function/u_uac2.h b/drivers/usb/gadget/function/u_uac2.h
> > index b503571..13589c3 100644
> > --- a/drivers/usb/gadget/function/u_uac2.h
> > +++ b/drivers/usb/gadget/function/u_uac2.h
> > @@ -21,6 +21,7 @@
> > #define UAC2_DEF_CCHMASK 0x3
> > #define UAC2_DEF_CSRATE 64000
> > #define UAC2_DEF_CSSIZE 2
> > +#define UAC2_DEF_CSYNC USB_ENDPOINT_SYNC_ASYNC
> > #define UAC2_DEF_REQ_NUM 2
> >
> > struct f_uac2_opts {
> > @@ -31,6 +32,7 @@ struct f_uac2_opts {
> > int c_chmask;
> > int c_srate;
> > int c_ssize;
> > + int c_sync;
> > int req_number;
> > bool bound;
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 0/3] UAC2 Gadget: feedback endpoint support
2020-11-26 13:16 ` Jerome Brunet
2020-11-26 13:44 ` Pavel Hofman
@ 2020-12-04 14:35 ` Ruslan Bilovol
1 sibling, 0 replies; 31+ messages in thread
From: Ruslan Bilovol @ 2020-12-04 14:35 UTC (permalink / raw)
To: Jerome Brunet; +Cc: Glenn Schmottlach, Peter Chen, balbi, linux-usb
On Thu, Nov 26, 2020 at 3:16 PM Jerome Brunet <jbrunet@baylibre.com> wrote:
>
>
> On Sun 22 Nov 2020 at 20:51, Ruslan Bilovol <ruslan.bilovol@gmail.com> wrote:
>
> > On Fri, Nov 13, 2020 at 5:35 PM Glenn Schmottlach
> > <gschmottlach@gmail.com> wrote:
> >>
> >> On Thu, Nov 12, 2020 at 6:20 PM Ruslan Bilovol <ruslan.bilovol@gmail.com> wrote:
> >> >
> >> > On Wed, Nov 11, 2020 at 11:30 AM Peter Chen <peter.chen@nxp.com> wrote:
> >> > >
> >> > > On 20-11-08 02:18:28, Ruslan Bilovol wrote:
> >> > > > Current UAC2 gadget implements capture/sync paths
> >> > > > as two USB ISO ASYNC endpoints (IN and OUT).
> >> > > >
> >> > > > This violates USB spec which says that ISO ASYNC OUT endpoint
> >> > > > should have feedback companion endpoint.
> >> > > > See USB2.0 spec "5.12.4.1 Synchronization Type": asynchronous
> >> > > > sink provides explicit feedback (isochronous pipe).
> >> > > > Interesting that for ISO ASYNC *IN* endpoint respective
> >> > > > feedback isn't required since source provides implicit
> >> > > > feedforward (data stream).
> >> > > >
> >> > > > While it's not an issue if UAC2 Gadget is connected to
> >> > > > Linux host (Linux ignores missing feedback endpoint),
> >> > > > with other hosts like Windows or MacOS the UAC2 Gadget
> >> > > > isn't enumerated due to missing feedback endpoint.
> >> > > >
> >> > > > This patch series adds feedback endpoint support to
> >> > > > UAC2 function, new control to UAC2 mixer which can
> >> > > > be used by userspace tools (like alsaloop from alsa-utils)
> >> > > > for updating feedback frequency reported to the host.
> >> > > > This is useful for usecases when UAC2 Gadget's audio
> >> > > > samples are played to another codec or audio card
> >> > > > with its own internal freerunning clock so host can
> >> > > > be notified that more/less samples are required.
> >> > > >
> >> > > > The alsaloop tool requires some (relatively small)
> >> > > > modifications in order to start support driving
> >> > > > feedback frequency through UAC2 mixer control.
> >> > > > That change will be sent as a separate patch
> >> > > > to ALSA community.
> >> > > >
> >> > > > Also added ability to switch ISO ASYNC OUT endpoint into
> >> > > > adaptive endpoint which doesn't require feedback endpoint
> >> > > > (as per USB spec).
> >> > > >
> >> > > > Ruslan Bilovol (3):
> >> > > > usb: gadget: f_uac2/u_audio: add feedback endpoint support
> >> > > > usb: gadget: f_uac2: add adaptive sync support for capture
> >> > > > usb: gadget: u_audio: add real feedback implementation
> >> > >
> >> > > Hi Ruslan,
> >> > >
> >> > > I applied your patches, but WIN10 still can't recognize it well.
> >> > > The UAC1 is OK for WIN10 with the below same configuration.
> >> > > Any debug information you would like to know to check it?
> >> > >
> >> > >
> >> > > if [ "$FUNC" == "uac2" ]; then
> >> > > mkdir functions/$FUNC".0"
> >> > > echo 2 > functions/$FUNC".0"/p_ssize
> >> > > echo 48000 > functions/$FUNC".0"/p_srate
> >> > > echo 3 > functions/$FUNC".0"/p_chmask
> >> > >
> >> > > echo 2 > functions/$FUNC".0"/c_ssize
> >> > > echo 48000 > functions/$FUNC".0"/c_srate
> >> > > echo 3 > functions/$FUNC".0"/c_chmask
> >> > > #echo 4 > functions/$FUNC".0"/req_number
> >> > > ln -s functions/$FUNC".0" configs/c.1
> >> > > echo high-speed > /sys/kernel/config/usb_gadget/g1/max_speed
> >> > > fi
> >> > >
> >> >
> >> > Hmm... I just tested below config and it works fine with my Win10.
> >> > The only thing I noticed is Windows doesn't enable UAC2 gadget
> >> > by default, but this can be done from Win10 sound settings
> >> >
> >> > --------------------------------->8--------------------------------------
> >> > mkdir cfg
> >> > mount none cfg -t configfs
> >> > mkdir cfg/usb_gadget/g1
> >> > cd cfg/usb_gadget/g1
> >> > mkdir configs/c.1
> >> > mkdir functions/uac2.0
> >> >
> >> > # 44.1 kHz sample rate
> >> > echo 44100 > functions/uac2.0/c_srate
> >> > echo 44100 > functions/uac2.0/p_srate
> >> >
> >> > mkdir strings/0x409
> >> > mkdir configs/c.1/strings/0x409
> >> > echo 0x0101 > idProduct
> >> > echo 0x1d6b > idVendor
> >> > echo my-serial-num > strings/0x409/serialnumber
> >> > echo my-manufacturer > strings/0x409/manufacturer
> >> > echo "Test gadget" > strings/0x409/product
> >> > echo "Conf 1" > configs/c.1/strings/0x409/configuration
> >> > echo 120 > configs/c.1/MaxPower
> >> > ln -s functions/uac2.0 configs/c.1
> >> > echo musb-hdrc.0 > UDC
> >> > --------------------------------->8--------------------------------------
> >> >
> >> > Thanks,
> >> > Ruslan
> >>
> >> Hi Ruslan -
> >>
> >> With your configuration (above) Win10 was able to recognize and load
> >> the driver. What appears to prevent my configuration from loading is
> >> the fact that I selected 48K as my sample rate and my capture/playback
> >> channel mask includes more than two (2) channels. If I take your
> >> configuration and change the sample rate (c_srate/p_srate) to 48000
> >> Windows will fail to load the driver. Likewise, setting the
> >> c_chmask/p_chmask to 7 (three channels) will also cause the driver to
> >> fail to load.
> >>
> >> You mentioned there is an option in Win10 Sound Settings to "enable"
> >> UAC2 by default. I cannot find that option and I wonder if this is
> >> what is preventing me from changing the sample rate or channel mask?
> >> Could Windows be treating my audio gadget as a UAC1 device rather than
> >> a fully multi-channel audio device (even though the usbaudio2.sys
> >> driver is loaded)? Have you tried other configurations to verify your
> >> Win10 box supports more than two channels and a 44.1K sample rate? I
> >> look forward to your feedback and any suggestions you might offer.
> >>
> >
> > I was able to reproduce the issue and prepared a patch below, please
> > try it and see if it fixes the issue.
> >
> > Maximum data rates that I used were (AFAIR) 8 channel 192kHz/32bit,
> > but there is another issue with high data rate if someone uses so many
> > channels, very high sampling frequency or sample size that data can't
> > fit into allowed (by USB spec) max packet size of endpoint. In this case
> > need to decrease bInterval of endpoint.
> > I test it in a high-performance audio case so always use a minimal possible
> > bInterval (set to '1').
> > Of course in the future that need to be calculated dynamically depending on
> > the UAC2 settings
> >
> > Please test patches below and see it it helps
> >
> > ---------------------------------------------8<----------------------------------------
> > From 51516435bbf2486574ec7bc9fd4726677cd474a4 Mon Sep 17 00:00:00 2001
> > From: Ruslan Bilovol <ruslan.bilovol@gmail.com>
> > Date: Sun, 22 Nov 2020 21:05:38 +0200
> > Subject: [PATCH] usb: gadget: f_uac2: always increase endpoint max_packet_size
> > by one audio slot
> >
> > As per UAC2 Audio Data Formats spec (2.3.1.1 USB Packets),
> > if the sampling rate is a constant, the allowable variation
> > of number of audio slots per virtual frame is +/- 1 audio slot.
> >
> > It means that endpoint should be able to accept/send +1 audio
> > slot.
> >
> > Previous endpoint max_packet_size calculation code
> > was adding sometimes +1 audio slot due to DIV_ROUND_UP
> > behaviour which was rounding up to closest integer.
> > However this doesn't work if the numbers are divisible.
> >
> > It had no any impact with Linux hosts which ignore
> > this issue, but in case of more strict Windows it
> > caused rejected enumeration
> >
> > Thus always add +1 audio slot to endpoint's max packet size
> >
> > Signed-off-by: Ruslan Bilovol <ruslan.bilovol@gmail.com>
> > ---
> > drivers/usb/gadget/function/f_uac2.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/usb/gadget/function/f_uac2.c
> > b/drivers/usb/gadget/function/f_uac2.c
> > index ebdb2a1..1698587 100644
> > --- a/drivers/usb/gadget/function/f_uac2.c
> > +++ b/drivers/usb/gadget/function/f_uac2.c
> > @@ -486,7 +486,7 @@ static void set_ep_max_packet_size(const struct
> > f_uac2_opts *uac2_opts,
> > }
> >
> > max_packet_size = num_channels(chmask) * ssize *
> > - DIV_ROUND_UP(srate, factor / (1 << (ep_desc->bInterval - 1)));
> > + ((srate / (factor / (1 << (ep_desc->bInterval - 1)))) + 1);
> >
> > if (!is_playback && (uac2_opts->c_sync == USB_ENDPOINT_SYNC_ASYNC))
> > max_packet_size = max_packet_size * FBACK_FREQ_MAX / 100;
>
> I think "reserving" 20% additional bandwidth is a bit extreme.
> In the end, the purpose is composate the drift which appears between
> different XTALs
That isn't extreme, I checked snd-usb ALSA driver before selecting number
and it assumes the drift is between -25% ... +50% so +/-20% should work
fine here (see snd_usb_handle_sync_urb() in sound/usb/endpoint.c).
Since our UAC2 gadget is a kind of "reversed" USB audio card, that also
works fine with alsaloop connecting it to other audio cards in the system.
>
> Allocating bandwidth for 1 more sample than the stream should require
> (49 instead of 48 for 48kHz with 4 bIntervals) should allow to
> compensate any realistic drift, don't you think ?
With cheap USB Audio adapters I saw +/-100 Hz drift on 44100 Hz
Anyway I don't think this can cause any problem since it is OK as
per UAC2 spec and hosts (at least Linux) can handle such values
Thanks,
Ruslan
>
> > --
> > 1.9.1
> >
> >
> > ---------------------------------------------8<----------------------------------------
> > From c8f2f2b414af672ec40841e75fb1ea761ae29122 Mon Sep 17 00:00:00 2001
> > From: Ruslan Bilovol <ruslan.bilovol@gmail.com>
> > Date: Sun, 16 Feb 2020 22:40:23 +0200
> > Subject: [PATCH] usb: gadget: f_uac2: change bInterval to 1 for
> > 8ch/24bit/44.1kHz
> >
> > With bInterval=4 one audio slot in case of 8ch/24bit/44.1kHz
> > exeeds MaxPacket size of ISO endpoint (>1024) and can't be
> > transferred in time.
> > USB spec ("5.6.3 Isochronous Transfer Packet Size Constraints")
> > says if we need to transfer more than 1024 it is a high speed, high
> > bandwidth endpoint and should have bInterval=1.
> >
> > In the future, bInterval should be dynamically calculated
> > depending on bandwith requirementes
>
> Probably better to do it from the start to avoid breaking stuff for
> people who have been using the gadget with the current badnwidth so far
That is an existing issue with UAC2 driver since its initial implementation,
I'm going to fix it in the near future as a separate patch. The issue is not
related to the feedback synchronization support patch.
I actually developed a few patches (not published yet) to check user
parameters and refuse to start UAC1/2 gadget if they are wrong
(currently UAC1/2 gagdet accepts incorrect parameters and then
silently fails to work correctly)
Thanks,
Ruslan
>
> >
> > Signed-off-by: Ruslan Bilovol <ruslan.bilovol@gmail.com>
> > ---
> > drivers/usb/gadget/function/f_uac2.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/usb/gadget/function/f_uac2.c
> > b/drivers/usb/gadget/function/f_uac2.c
> > index 3633df6..fb9b875 100644
> > --- a/drivers/usb/gadget/function/f_uac2.c
> > +++ b/drivers/usb/gadget/function/f_uac2.c
> > @@ -281,7 +281,7 @@ enum {
> >
> > .bmAttributes = USB_ENDPOINT_XFER_ISOC | USB_ENDPOINT_SYNC_ASYNC,
> > .wMaxPacketSize = cpu_to_le16(1024),
> > - .bInterval = 4,
> > + .bInterval = 1,
> > };
> >
> > /* CS AS ISO OUT Endpoint */
> > @@ -358,7 +358,7 @@ enum {
> >
> > .bmAttributes = USB_ENDPOINT_XFER_ISOC | USB_ENDPOINT_SYNC_ASYNC,
> > .wMaxPacketSize = cpu_to_le16(1024),
> > - .bInterval = 4,
> > + .bInterval = 1,
> > };
> >
> > /* CS AS ISO IN Endpoint */
>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 0/3] UAC2 Gadget: feedback endpoint support
2020-11-26 13:44 ` Pavel Hofman
@ 2020-12-04 14:39 ` Ruslan Bilovol
2020-12-04 18:08 ` Pavel Hofman
0 siblings, 1 reply; 31+ messages in thread
From: Ruslan Bilovol @ 2020-12-04 14:39 UTC (permalink / raw)
To: Pavel Hofman
Cc: Jerome Brunet, Glenn Schmottlach, Peter Chen, balbi, linux-usb
On Thu, Nov 26, 2020 at 3:44 PM Pavel Hofman <pavel.hofman@ivitera.com> wrote:
>
> Dne 26. 11. 20 v 14:16 Jerome Brunet napsal(a):
> >
> >> Maximum data rates that I used were (AFAIR) 8 channel 192kHz/32bit,
> >> but there is another issue with high data rate if someone uses so many
> >> channels, very high sampling frequency or sample size that data can't
> >> fit into allowed (by USB spec) max packet size of endpoint. In this case
> >> need to decrease bInterval of endpoint.
>
> Should anyone test the patches with RPi4 dwc2 as the gadget, please note
> the recently accepted patch
> https://patchwork.kernel.org/project/linux-usb/patch/e9e7d070-593c-122f-3a5c-2435bb147ab2@ivitera.com/
> which allows using full 1024 bytes maxpacketsize on EP OUT. It is not an
> enumeration issue, but the old (= existing) RX FIFO size drops data with
> packet sizes above 960 bytes.
Thanks for bringing this to our attention. Does it affect RPi 3 as well?
I'm going to test feedback feature on this board
Regards,
Ruslan
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 0/3] UAC2 Gadget: feedback endpoint support
2020-12-04 14:39 ` Ruslan Bilovol
@ 2020-12-04 18:08 ` Pavel Hofman
0 siblings, 0 replies; 31+ messages in thread
From: Pavel Hofman @ 2020-12-04 18:08 UTC (permalink / raw)
To: Ruslan Bilovol
Cc: Jerome Brunet, Glenn Schmottlach, Peter Chen, balbi, linux-usb
Dne 04. 12. 20 v 15:39 Ruslan Bilovol napsal(a):
> On Thu, Nov 26, 2020 at 3:44 PM Pavel Hofman <pavel.hofman@ivitera.com> wrote:
>>
>> Dne 26. 11. 20 v 14:16 Jerome Brunet napsal(a):
>>>
>>>> Maximum data rates that I used were (AFAIR) 8 channel 192kHz/32bit,
>>>> but there is another issue with high data rate if someone uses so many
>>>> channels, very high sampling frequency or sample size that data can't
>>>> fit into allowed (by USB spec) max packet size of endpoint. In this case
>>>> need to decrease bInterval of endpoint.
>>
>> Should anyone test the patches with RPi4 dwc2 as the gadget, please note
>> the recently accepted patch
>> https://patchwork.kernel.org/project/linux-usb/patch/e9e7d070-593c-122f-3a5c-2435bb147ab2@ivitera.com/
>> which allows using full 1024 bytes maxpacketsize on EP OUT. It is not an
>> enumeration issue, but the old (= existing) RX FIFO size drops data with
>> packet sizes above 960 bytes.
>
> Thanks for bringing this to our attention. Does it affect RPi 3 as well?
> I'm going to test feedback feature on this board
It configures the dwc2 IP of the broadcom which IIRC is in all RPis. The
affected files bcm283x-rpi-usb-otg.dtsi and
bcm283x-rpi-usb-peripheral.dtsi do not seem RPi4 specific. But I had
only RPi4 to analyze the problem and check the fix.
Ruslan, please what do you think about the existing behaviour of gaudio
in case of USB stream disconnection? The elapsed-period callback stops
being called and the user space will eventually time out. How about
closing the PCM stream immediately in case of a problem in the USB
layer? E.g. some SPDIF receivers do that in case of change in the
incoming samplerate
https://github.com/torvalds/linux/blob/master/sound/i2c/other/ak4117.c#L503
.
Thanks a lot for the great work.
With regards,
Pavel.
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 0/3] UAC2 Gadget: feedback endpoint support
2020-12-01 21:43 ` Glenn Schmottlach
2020-12-02 22:04 ` Glenn Schmottlach
@ 2020-12-10 12:46 ` Ruslan Bilovol
1 sibling, 0 replies; 31+ messages in thread
From: Ruslan Bilovol @ 2020-12-10 12:46 UTC (permalink / raw)
To: Glenn Schmottlach; +Cc: Peter Chen, balbi, linux-usb
On Tue, Dec 1, 2020 at 11:44 PM Glenn Schmottlach
<gschmottlach@gmail.com> wrote:
>
> On Sat, Nov 28, 2020 at 6:26 PM Ruslan Bilovol <ruslan.bilovol@gmail.com> wrote:
> >
> > On Wed, Nov 25, 2020 at 9:29 PM Glenn Schmottlach
> > <gschmottlach@gmail.com> wrote:
> > >
> > > On Sun, Nov 22, 2020 at 2:52 PM Ruslan Bilovol <ruslan.bilovol@gmail.com> wrote:
> > > >
> > > > On Fri, Nov 13, 2020 at 5:35 PM Glenn Schmottlach
> > > > <gschmottlach@gmail.com> wrote:
> > > > >
> > > > > On Thu, Nov 12, 2020 at 6:20 PM Ruslan Bilovol <ruslan.bilovol@gmail.com> wrote:
> > > > > >
> > > > > > On Wed, Nov 11, 2020 at 11:30 AM Peter Chen <peter.chen@nxp.com> wrote:
> > > > > > >
> > > > > > > On 20-11-08 02:18:28, Ruslan Bilovol wrote:
> > > > > > > > Current UAC2 gadget implements capture/sync paths
> > > > > > > > as two USB ISO ASYNC endpoints (IN and OUT).
> > > > > > > >
> > > > > > > > This violates USB spec which says that ISO ASYNC OUT endpoint
> > > > > > > > should have feedback companion endpoint.
> > > > > > > > See USB2.0 spec "5.12.4.1 Synchronization Type": asynchronous
> > > > > > > > sink provides explicit feedback (isochronous pipe).
> > > > > > > > Interesting that for ISO ASYNC *IN* endpoint respective
> > > > > > > > feedback isn't required since source provides implicit
> > > > > > > > feedforward (data stream).
> > > > > > > >
> > > > > > > > While it's not an issue if UAC2 Gadget is connected to
> > > > > > > > Linux host (Linux ignores missing feedback endpoint),
> > > > > > > > with other hosts like Windows or MacOS the UAC2 Gadget
> > > > > > > > isn't enumerated due to missing feedback endpoint.
> > > > > > > >
> > > > > > > > This patch series adds feedback endpoint support to
> > > > > > > > UAC2 function, new control to UAC2 mixer which can
> > > > > > > > be used by userspace tools (like alsaloop from alsa-utils)
> > > > > > > > for updating feedback frequency reported to the host.
> > > > > > > > This is useful for usecases when UAC2 Gadget's audio
> > > > > > > > samples are played to another codec or audio card
> > > > > > > > with its own internal freerunning clock so host can
> > > > > > > > be notified that more/less samples are required.
> > > > > > > >
> > > > > > > > The alsaloop tool requires some (relatively small)
> > > > > > > > modifications in order to start support driving
> > > > > > > > feedback frequency through UAC2 mixer control.
> > > > > > > > That change will be sent as a separate patch
> > > > > > > > to ALSA community.
> > > > > > > >
> > > > > > > > Also added ability to switch ISO ASYNC OUT endpoint into
> > > > > > > > adaptive endpoint which doesn't require feedback endpoint
> > > > > > > > (as per USB spec).
> > > > > > > >
> > > > > > > > Ruslan Bilovol (3):
> > > > > > > > usb: gadget: f_uac2/u_audio: add feedback endpoint support
> > > > > > > > usb: gadget: f_uac2: add adaptive sync support for capture
> > > > > > > > usb: gadget: u_audio: add real feedback implementation
> > > > > > >
> > > > > > > Hi Ruslan,
> > > > > > >
> > > > > > > I applied your patches, but WIN10 still can't recognize it well.
> > > > > > > The UAC1 is OK for WIN10 with the below same configuration.
> > > > > > > Any debug information you would like to know to check it?
> > > > > > >
> > > > > > >
> > > > > > > if [ "$FUNC" == "uac2" ]; then
> > > > > > > mkdir functions/$FUNC".0"
> > > > > > > echo 2 > functions/$FUNC".0"/p_ssize
> > > > > > > echo 48000 > functions/$FUNC".0"/p_srate
> > > > > > > echo 3 > functions/$FUNC".0"/p_chmask
> > > > > > >
> > > > > > > echo 2 > functions/$FUNC".0"/c_ssize
> > > > > > > echo 48000 > functions/$FUNC".0"/c_srate
> > > > > > > echo 3 > functions/$FUNC".0"/c_chmask
> > > > > > > #echo 4 > functions/$FUNC".0"/req_number
> > > > > > > ln -s functions/$FUNC".0" configs/c.1
> > > > > > > echo high-speed > /sys/kernel/config/usb_gadget/g1/max_speed
> > > > > > > fi
> > > > > > >
> > > > > >
> > > > > > Hmm... I just tested below config and it works fine with my Win10.
> > > > > > The only thing I noticed is Windows doesn't enable UAC2 gadget
> > > > > > by default, but this can be done from Win10 sound settings
> > > > > >
> > > > > > --------------------------------->8--------------------------------------
> > > > > > mkdir cfg
> > > > > > mount none cfg -t configfs
> > > > > > mkdir cfg/usb_gadget/g1
> > > > > > cd cfg/usb_gadget/g1
> > > > > > mkdir configs/c.1
> > > > > > mkdir functions/uac2.0
> > > > > >
> > > > > > # 44.1 kHz sample rate
> > > > > > echo 44100 > functions/uac2.0/c_srate
> > > > > > echo 44100 > functions/uac2.0/p_srate
> > > > > >
> > > > > > mkdir strings/0x409
> > > > > > mkdir configs/c.1/strings/0x409
> > > > > > echo 0x0101 > idProduct
> > > > > > echo 0x1d6b > idVendor
> > > > > > echo my-serial-num > strings/0x409/serialnumber
> > > > > > echo my-manufacturer > strings/0x409/manufacturer
> > > > > > echo "Test gadget" > strings/0x409/product
> > > > > > echo "Conf 1" > configs/c.1/strings/0x409/configuration
> > > > > > echo 120 > configs/c.1/MaxPower
> > > > > > ln -s functions/uac2.0 configs/c.1
> > > > > > echo musb-hdrc.0 > UDC
> > > > > > --------------------------------->8--------------------------------------
> > > > > >
> > > > > > Thanks,
> > > > > > Ruslan
> > > > >
> > > > > Hi Ruslan -
> > > > >
> > > > > With your configuration (above) Win10 was able to recognize and load
> > > > > the driver. What appears to prevent my configuration from loading is
> > > > > the fact that I selected 48K as my sample rate and my capture/playback
> > > > > channel mask includes more than two (2) channels. If I take your
> > > > > configuration and change the sample rate (c_srate/p_srate) to 48000
> > > > > Windows will fail to load the driver. Likewise, setting the
> > > > > c_chmask/p_chmask to 7 (three channels) will also cause the driver to
> > > > > fail to load.
> > > > >
> > > > > You mentioned there is an option in Win10 Sound Settings to "enable"
> > > > > UAC2 by default. I cannot find that option and I wonder if this is
> > > > > what is preventing me from changing the sample rate or channel mask?
> > > > > Could Windows be treating my audio gadget as a UAC1 device rather than
> > > > > a fully multi-channel audio device (even though the usbaudio2.sys
> > > > > driver is loaded)? Have you tried other configurations to verify your
> > > > > Win10 box supports more than two channels and a 44.1K sample rate? I
> > > > > look forward to your feedback and any suggestions you might offer.
> > > > >
> > > >
> > > > I was able to reproduce the issue and prepared a patch below, please
> > > > try it and see if it fixes the issue.
> > > >
> > > > Maximum data rates that I used were (AFAIR) 8 channel 192kHz/32bit,
> > > > but there is another issue with high data rate if someone uses so many
> > > > channels, very high sampling frequency or sample size that data can't
> > > > fit into allowed (by USB spec) max packet size of endpoint. In this case
> > > > need to decrease bInterval of endpoint.
> > > > I test it in a high-performance audio case so always use a minimal possible
> > > > bInterval (set to '1').
> > > > Of course in the future that need to be calculated dynamically depending on
> > > > the UAC2 settings
> > > >
> > > > Please test patches below and see it it helps
> > > >
> > > > ---------------------------------------------8<----------------------------------------
> > > > From 51516435bbf2486574ec7bc9fd4726677cd474a4 Mon Sep 17 00:00:00 2001
> > > > From: Ruslan Bilovol <ruslan.bilovol@gmail.com>
> > > > Date: Sun, 22 Nov 2020 21:05:38 +0200
> > > > Subject: [PATCH] usb: gadget: f_uac2: always increase endpoint max_packet_size
> > > > by one audio slot
> > > >
> > > > As per UAC2 Audio Data Formats spec (2.3.1.1 USB Packets),
> > > > if the sampling rate is a constant, the allowable variation
> > > > of number of audio slots per virtual frame is +/- 1 audio slot.
> > > >
> > > > It means that endpoint should be able to accept/send +1 audio
> > > > slot.
> > > >
> > > > Previous endpoint max_packet_size calculation code
> > > > was adding sometimes +1 audio slot due to DIV_ROUND_UP
> > > > behaviour which was rounding up to closest integer.
> > > > However this doesn't work if the numbers are divisible.
> > > >
> > > > It had no any impact with Linux hosts which ignore
> > > > this issue, but in case of more strict Windows it
> > > > caused rejected enumeration
> > > >
> > > > Thus always add +1 audio slot to endpoint's max packet size
> > > >
> > > > Signed-off-by: Ruslan Bilovol <ruslan.bilovol@gmail.com>
> > > > ---
> > > > drivers/usb/gadget/function/f_uac2.c | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/usb/gadget/function/f_uac2.c
> > > > b/drivers/usb/gadget/function/f_uac2.c
> > > > index ebdb2a1..1698587 100644
> > > > --- a/drivers/usb/gadget/function/f_uac2.c
> > > > +++ b/drivers/usb/gadget/function/f_uac2.c
> > > > @@ -486,7 +486,7 @@ static void set_ep_max_packet_size(const struct
> > > > f_uac2_opts *uac2_opts,
> > > > }
> > > >
> > > > max_packet_size = num_channels(chmask) * ssize *
> > > > - DIV_ROUND_UP(srate, factor / (1 << (ep_desc->bInterval - 1)));
> > > > + ((srate / (factor / (1 << (ep_desc->bInterval - 1)))) + 1);
> > > >
> > > > if (!is_playback && (uac2_opts->c_sync == USB_ENDPOINT_SYNC_ASYNC))
> > > > max_packet_size = max_packet_size * FBACK_FREQ_MAX / 100;
> > > > --
> > > > 1.9.1
> > > >
> > > >
> > > > ---------------------------------------------8<----------------------------------------
> > > > From c8f2f2b414af672ec40841e75fb1ea761ae29122 Mon Sep 17 00:00:00 2001
> > > > From: Ruslan Bilovol <ruslan.bilovol@gmail.com>
> > > > Date: Sun, 16 Feb 2020 22:40:23 +0200
> > > > Subject: [PATCH] usb: gadget: f_uac2: change bInterval to 1 for
> > > > 8ch/24bit/44.1kHz
> > > >
> > > > With bInterval=4 one audio slot in case of 8ch/24bit/44.1kHz
> > > > exeeds MaxPacket size of ISO endpoint (>1024) and can't be
> > > > transferred in time.
> > > > USB spec ("5.6.3 Isochronous Transfer Packet Size Constraints")
> > > > says if we need to transfer more than 1024 it is a high speed, high
> > > > bandwidth endpoint and should have bInterval=1.
> > > >
> > > > In the future, bInterval should be dynamically calculated
> > > > depending on bandwith requirementes
> > > >
> > > > Signed-off-by: Ruslan Bilovol <ruslan.bilovol@gmail.com>
> > > > ---
> > > > drivers/usb/gadget/function/f_uac2.c | 4 ++--
> > > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/usb/gadget/function/f_uac2.c
> > > > b/drivers/usb/gadget/function/f_uac2.c
> > > > index 3633df6..fb9b875 100644
> > > > --- a/drivers/usb/gadget/function/f_uac2.c
> > > > +++ b/drivers/usb/gadget/function/f_uac2.c
> > > > @@ -281,7 +281,7 @@ enum {
> > > >
> > > > .bmAttributes = USB_ENDPOINT_XFER_ISOC | USB_ENDPOINT_SYNC_ASYNC,
> > > > .wMaxPacketSize = cpu_to_le16(1024),
> > > > - .bInterval = 4,
> > > > + .bInterval = 1,
> > > > };
> > > >
> > > > /* CS AS ISO OUT Endpoint */
> > > > @@ -358,7 +358,7 @@ enum {
> > > >
> > > > .bmAttributes = USB_ENDPOINT_XFER_ISOC | USB_ENDPOINT_SYNC_ASYNC,
> > > > .wMaxPacketSize = cpu_to_le16(1024),
> > > > - .bInterval = 4,
> > > > + .bInterval = 1,
> > > > };
> > > >
> > > > /* CS AS ISO IN Endpoint */
> > > > --
> > > > 1.9.1
> > > >
> > > >
> > > > ---------------------------------------------8<----------------------------------------
> > > >
> > > > Thanks,
> > > > Ruslan
> > >
> > > Hi Ruslan -
> > >
> > > I have had some mixed success with your additional patches. First, the
> >
> > That's good news, thanks for testing!
> >
> > > UAC2 gadget does correctly register with Windows 10 but only for 1-31
> > > channels. For a fully specified channel map (32 channels):
> > >
> > > c_chmask = 0xFFFFFFFF ( Does not load)
> > > p_chmask = 0xFFFFFFFF ( Does not load)
> > >
> > > the driver fails to load (code=10). This seems similar to the prior
> > > problem. Could there be another off-by-one problem here?
> > >
> > > With a 31 channel mask the Windows 10 usbaudio2.sys loads successfully, e.g.
> > >
> > > c_chmask = 0x7FFFFFFF (loads)
> > > p_chmask = 0x7FFFFFFF (loads)
> >
> > That's correct behavior. UAC2 spec says that bits D27..D30 are reserved.
> > Also bit D31 is mutually exclusive with other bits and means 'raw data'.
> > It means that with current UAC2 gadget implementation you can enable
> > only 27 channels.
> >
> > It seems like it's possible to add more channels by adding more audio
> > clusters but that requires UAC2 driver modifications. Also it's not clear
> > whether it's implemented in Host OS drivers
> >
> > I personally tested as many as 8x8 playback/capture channels simultaneously.
> >
> > >
> > > Once Windows 10 loads the driver successfully, however, I can't seem
> > > to select the device through Audacity on Windows. The device appears
> > > in the Windows "Device Manager" under "Sound, video and game
> > > controllers -> Source/Sink". How are you testing audio
> > > playback/capture under Windows? Perhaps my test approach is wrong?
> >
> > You need to go to sound settings -> manage sound devices and enable
> > UAC2 gadget there, after that it will appear in the system.
> >
> > I also used Audacity for testing this and it works fine for me
> >
> > Thanks,
> > Ruslan
> >
> > >
> > > I used the same approach on my Linux host where the USB UAC2 device
> > > appears as an ALSA device and I use speaker-test on the target
> > > (BeagleBone Black) to play and then capture the audio using Audacity
> > > on the Linux host. I was hoping to do the same or something similar
> > > under Windows.
> > >
> > > Below is the script I've been using on the target to configure my UAC2 device:
> > >
> > > mkdir -p "/sys/kernel/config/usb_gadget/g_multi"
> > > echo "${idVendor:-0x11F5}" > "/sys/kernel/config/usb_gadget/g_multi/idVendor"
> > > echo "${idProduct:-0x0009}" > "/sys/kernel/config/usb_gadget/g_multi/idProduct"
> > > echo "${bcdDevice:-0x0001}" > "/sys/kernel/config/usb_gadget/g_multi/bcdDevice"
> > > echo "${bDeviceClass:-0x00}" >
> > > "/sys/kernel/config/usb_gadget/g_multi/bDeviceClass"
> > > echo "${bDeviceSubClass:-0x00}" >
> > > "/sys/kernel/config/usb_gadget/g_multi/bDeviceSubClass"
> > > echo "${bDeviceProtocol:-0x00}" >
> > > "/sys/kernel/config/usb_gadget/g_multi/bDeviceProtocol"
> > > echo "${bcdUSB:-0x0200}" > "/sys/kernel/config/usb_gadget/g_multi/bcdUSB"
> > > echo "${bMaxPacketSize0:-0x00}" >
> > > "/sys/kernel/config/usb_gadget/g_multi/bMaxPacketSize0"
> > > mkdir -p "/sys/kernel/config/usb_gadget/g_multi/strings/0x409"
> > > echo "${serialnumber:-000000000000}" >
> > > "/sys/kernel/config/usb_gadget/g_multi/strings/0x409/serialnumber"
> > > echo "${product:-UAC2}" >
> > > "/sys/kernel/config/usb_gadget/g_multi/strings/0x409/product"
> > > echo "${manufacturer:-www.acme.com}" >
> > > "/sys/kernel/config/usb_gadget/g_multi/strings/0x409/manufacturer"
> > > mkdir -p "/sys/kernel/config/usb_gadget/g_multi/functions/uac2.usb0"
> > > echo "${req_number:-4}" >
> > > "/sys/kernel/config/usb_gadget/g_multi/functions/uac2.usb0/req_number"
> > > echo "${c_ssize:-4}" >
> > > "/sys/kernel/config/usb_gadget/g_multi/functions/uac2.usb0/c_ssize"
> > > echo "${c_srate:-48000}" >
> > > "/sys/kernel/config/usb_gadget/g_multi/functions/uac2.usb0/c_srate"
> > > echo "${c_chmask:-0x00000003}" >
> > > "/sys/kernel/config/usb_gadget/g_multi/functions/uac2.usb0/c_chmask"
> > > echo "${c_sync:-asynchronous}" >
> > > "/sys/kernel/config/usb_gadget/g_multi/functions/uac2.usb0/c_sync"
> > > echo "${p_ssize:-4}" >
> > > "/sys/kernel/config/usb_gadget/g_multi/functions/uac2.usb0/p_ssize"
> > > echo "${p_srate:-48000}" >
> > > "/sys/kernel/config/usb_gadget/g_multi/functions/uac2.usb0/p_srate"
> > > echo "${p_chmask:-0x00000003}" >
> > > "/sys/kernel/config/usb_gadget/g_multi/functions/uac2.usb0/p_chmask"
> > > mkdir -p "/sys/kernel/config/usb_gadget/g_multi/configs/c.1"
> > > echo "${bmAttributes:-0x80}" >
> > > "/sys/kernel/config/usb_gadget/g_multi/configs/c.1/bmAttributes"
> > > echo "${MaxPower:-500}" >
> > > "/sys/kernel/config/usb_gadget/g_multi/configs/c.1/MaxPower"
> > > mkdir -p "/sys/kernel/config/usb_gadget/g_multi/configs/c.1/strings/0x409"
> > > echo "${configuration:-BeagleBone Composite}" >
> > > "/sys/kernel/config/usb_gadget/g_multi/configs/c.1/strings/0x409/configuration"
> > > ln -s "/sys/kernel/config/usb_gadget/g_multi/functions/uac2.usb0"
> > > "/sys/kernel/config/usb_gadget/g_multi/configs/c.1/uac2.usb0"
> > > mkdir -p "/sys/kernel/config/usb_gadget/g_multi/os_desc"
> > > ln -s "//sys/kernel/config/usb_gadget/g_multi/configs/c.1"
> > > "/sys/kernel/config/usb_gadget/g_multi/os_desc/c.1"
> > > echo "${qw_sign:-MSFT100}" >
> > > "/sys/kernel/config/usb_gadget/g_multi/os_desc/qw_sign"
> > > echo "${b_vendor_code:-0x00}" >
> > > "/sys/kernel/config/usb_gadget/g_multi/os_desc/b_vendor_code"
> > > echo "${use:-1}" > "/sys/kernel/config/usb_gadget/g_multi/os_desc/use"
> > >
> > > basename /sys/class/udc/* > /sys/kernel/config/usb_gadget/g_multi/UDC
> > >
> > > This is only a two channel playback/capture configuration. I hoped to
> > > get this working under Windows before increasing my channel count to a
> > > full 32 channel mask with a 48K sample rate.
> > >
> > > I look forward to hearing your suggestions and possibly pointing out
> > > where I've gone astray.
> > >
> > > Thanks,
> > >
> > > Glenn
>
> Hi Ruslan -
>
> Thanks for the feedback but unfortunately I've experienced mixed
> results with the gadget UAC2 driver on both Windows/Linux. Let me
> describe my environment. My host platform is either a Linux Ubuntu
> 18.04 or Windows 10 laptop while the target environment is a
> BeagleBone Black (Linux beaglebone 5.4.74-g9574bba32a #1 PREEMPT). I'm
> testing two different scenarios:
>
> Scenario #1:
> BeagleBone Black (BBB) runs speaker-test generating a single channel
> (S32_LE) audio stream containing a 1KHz tone with a 48K sample rate,
> e.g.
>
> > speaker-test -D hw:1,0 -r 48000 -c 1 -f 1000 -F S32_LE -t sine
>
> The host laptop is running Audacity and recording the tone over the
> UAC2 adapter. On the Linux host the capture is correct and the tone is
> bit-perfect. On the Windows 10 the capture contains numerous missing
> samples which translates into a lot of audible pops and clicks.
>
> Scenario #2:
> The Linux/Windows host plays a single channel, 48K, S32_LE 1K sine
> tone to the target using either Audacity (on Windows) or 'aplay' (on
> Linux), e.g.
>
> > aplay -D hw:4,0 -c 1 -r 48000 -t wav tone_1k.wav (Linux)
>
> On the BBB target I use 'arecord' to record the tone to a RAM disk and
> then copy the recorded file back to the host where I can verify the
> quality of the recording. In both instances (e.g. using either Windows
> or Linux for playback) the recording on the target results in a
> captured file with missing samples and audible pops/clicks. In this
> scenario the UAC2 gadget is configured with c_sync == asynchronous. I
> wouldn't expect things to improve with c_sync == adaptive since you
> mentioned in your patch that it always reports back the nominal
> frequency to the host from the feedback endpoint.
>
> Do you have any suggestions that might explain (the above) behavior.
> Can you describe your test environment in more detail so that I can
> perhaps re-create it? What Linux target are you using with your tests?
> You mentioned you tested an 8x8 playback/capture scenario. Can you
> provide any details of how you performed this test and the method you
> used to confirm the audio quality for the capture/playback?
I also use BeagleBone black in my development.
I have two testing scenarios:
1) BeagleBone PC loopback (e.g. redirect play->capture in BBB), here
I play sine wave on PC to BBB and record back (simultaneously in
Audacity), it looks like:
Audiacity (PC playback) -> BBB capture -> alsaloop -> BBB playback ->
Audacity (PC capture)
2) I connect another USB audio card to BBB, and redirect audio samples
from PC through BBB UAC2 audio gadget to USB audio card, this path
looks like:
PC playback) -> BBB capture -> alsaloop -> BBB USB audio card
With #1 I tested simultaneous 8x8ch 24bit 44.1 kHz loopback (also 4x4ch
and 2x2 ch)
With #2 it's limited to USB audio card I have (usually I do 2ch 16bit 44.1 or
48 kHz)
I see you use 32bit samples, I tested it a few times before on the previous
patch set versions.
Can you try to check 2ch 16bit 44.1 or 48 kHz? I'm thinking if this issue is
limited to some combination of sampling rate, channels and sample resolution
Thanks,
Ruslan
>
> Thanks for any insights you might be able to offer . . .
>
> Glenn
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 0/3] UAC2 Gadget: feedback endpoint support
2020-12-03 10:09 ` Peter Chen
2020-12-03 22:07 ` Glenn Schmottlach
@ 2020-12-10 12:59 ` Ruslan Bilovol
2020-12-11 7:22 ` Peter Chen
1 sibling, 1 reply; 31+ messages in thread
From: Ruslan Bilovol @ 2020-12-10 12:59 UTC (permalink / raw)
To: Peter Chen; +Cc: Glenn Schmottlach, balbi, linux-usb
On Thu, Dec 3, 2020 at 12:09 PM Peter Chen <peter.chen@nxp.com> wrote:
>
> On 20-12-02 17:04:47, Glenn Schmottlach wrote:
> > On Tue, Dec 1, 2020 at 4:43 PM Glenn Schmottlach <gschmottlach@gmail.com> wrote:
> > > Hi Ruslan -
> > >
> > > Thanks for the feedback but unfortunately I've experienced mixed
> > > results with the gadget UAC2 driver on both Windows/Linux. Let me
> > > describe my environment. My host platform is either a Linux Ubuntu
> > > 18.04 or Windows 10 laptop while the target environment is a
> > > BeagleBone Black (Linux beaglebone 5.4.74-g9574bba32a #1 PREEMPT). I'm
> > > testing two different scenarios:
> > >
> > > Scenario #1:
> > > BeagleBone Black (BBB) runs speaker-test generating a single channel
> > > (S32_LE) audio stream containing a 1KHz tone with a 48K sample rate,
> > > e.g.
> > >
> > > > speaker-test -D hw:1,0 -r 48000 -c 1 -f 1000 -F S32_LE -t sine
> > >
> > > The host laptop is running Audacity and recording the tone over the
> > > UAC2 adapter. On the Linux host the capture is correct and the tone is
> > > bit-perfect. On the Windows 10 the capture contains numerous missing
> > > samples which translates into a lot of audible pops and clicks.
> > >
> > > Scenario #2:
> > > The Linux/Windows host plays a single channel, 48K, S32_LE 1K sine
> > > tone to the target using either Audacity (on Windows) or 'aplay' (on
> > > Linux), e.g.
> > >
> > > > aplay -D hw:4,0 -c 1 -r 48000 -t wav tone_1k.wav (Linux)
> > >
> > > On the BBB target I use 'arecord' to record the tone to a RAM disk and
> > > then copy the recorded file back to the host where I can verify the
> > > quality of the recording. In both instances (e.g. using either Windows
> > > or Linux for playback) the recording on the target results in a
> > > captured file with missing samples and audible pops/clicks. In this
> > > scenario the UAC2 gadget is configured with c_sync == asynchronous. I
> > > wouldn't expect things to improve with c_sync == adaptive since you
> > > mentioned in your patch that it always reports back the nominal
> > > frequency to the host from the feedback endpoint.
> > >
> > > Do you have any suggestions that might explain (the above) behavior.
> > > Can you describe your test environment in more detail so that I can
> > > perhaps re-create it? What Linux target are you using with your tests?
> > > You mentioned you tested an 8x8 playback/capture scenario. Can you
> > > provide any details of how you performed this test and the method you
> > > used to confirm the audio quality for the capture/playback?
> > >
> > > Thanks for any insights you might be able to offer . . .
> > >
> > > Glenn
> >
> > Hi Ruslan -
> >
> > This is a follow-up from my post yesterday. I recompiled my kernel
> > *WITHOUT* your UAC2 patches and repeated Scenario #2 where the Linux
> > PC plays a single channel tone to the BeagleBone Black where it's
> > recorded with 'arecord'. Yesterday, I recorded garbled audio on the
> > target but today, without any UAC2 kernel patches, the recorded audio
> > on the target is glitch-free and appears to be bit-perfect.
> >
> > This experiment leads me to believe your patches may be inadvertently
> > corrupting the data-path. Have you been able to repeat my experiment
> > and either confirm or refute my findings? I am interested to learn
> > more how you tested your patches and whether it's something I can
> > recreate here.
> >
> > Assuming we can sort out this data corruption issue, what are your
> > thoughts on how the Linux target device can properly provide the
> > Windows feedback endpoint with real frequency updates rather than the
> > constant nominal frequency. If I understood your patch notes correctly
> > it seems this is an outstanding issue that requires additional
> > attention. I'm a bit of a noob when it comes to how this might be
> > addressed.
> >
> > Thanks for your continued insights and support . . .
> >
> > Glenn
>
> Hi Glenn & Ruslan,
>
> Do you know why WIN10 can't recognized UAC2 device if I configure the
> sample rate as 48000HZ? Configuring sample rate as 44100HZ, the playback
> function would work well at my platforms (chipidea IP), no glitch is
> heard. At WIN10, I use Windows Media Player, at board side I use command:
That's known issue, Windows is more strict with UAC2 descriptors, try to apply
my patch "usb: gadget: f_uac2: always increase endpoint max_packet_size
by one audio slot" that I shared in this email:
https://www.spinics.net/lists/linux-usb/msg205077.html
Thanks,
Ruslan
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 0/3] UAC2 Gadget: feedback endpoint support
2020-12-10 12:59 ` Ruslan Bilovol
@ 2020-12-11 7:22 ` Peter Chen
0 siblings, 0 replies; 31+ messages in thread
From: Peter Chen @ 2020-12-11 7:22 UTC (permalink / raw)
To: Ruslan Bilovol; +Cc: Glenn Schmottlach, balbi, linux-usb
[-- Attachment #1: Type: text/plain, Size: 5574 bytes --]
On 20-12-10 14:59:24, Ruslan Bilovol wrote:
> On Thu, Dec 3, 2020 at 12:09 PM Peter Chen <peter.chen@nxp.com> wrote:
> >
> > On 20-12-02 17:04:47, Glenn Schmottlach wrote:
> > > On Tue, Dec 1, 2020 at 4:43 PM Glenn Schmottlach <gschmottlach@gmail.com> wrote:
> > > > Hi Ruslan -
> > > >
> > > > Thanks for the feedback but unfortunately I've experienced mixed
> > > > results with the gadget UAC2 driver on both Windows/Linux. Let me
> > > > describe my environment. My host platform is either a Linux Ubuntu
> > > > 18.04 or Windows 10 laptop while the target environment is a
> > > > BeagleBone Black (Linux beaglebone 5.4.74-g9574bba32a #1 PREEMPT). I'm
> > > > testing two different scenarios:
> > > >
> > > > Scenario #1:
> > > > BeagleBone Black (BBB) runs speaker-test generating a single channel
> > > > (S32_LE) audio stream containing a 1KHz tone with a 48K sample rate,
> > > > e.g.
> > > >
> > > > > speaker-test -D hw:1,0 -r 48000 -c 1 -f 1000 -F S32_LE -t sine
> > > >
> > > > The host laptop is running Audacity and recording the tone over the
> > > > UAC2 adapter. On the Linux host the capture is correct and the tone is
> > > > bit-perfect. On the Windows 10 the capture contains numerous missing
> > > > samples which translates into a lot of audible pops and clicks.
> > > >
> > > > Scenario #2:
> > > > The Linux/Windows host plays a single channel, 48K, S32_LE 1K sine
> > > > tone to the target using either Audacity (on Windows) or 'aplay' (on
> > > > Linux), e.g.
> > > >
> > > > > aplay -D hw:4,0 -c 1 -r 48000 -t wav tone_1k.wav (Linux)
> > > >
> > > > On the BBB target I use 'arecord' to record the tone to a RAM disk and
> > > > then copy the recorded file back to the host where I can verify the
> > > > quality of the recording. In both instances (e.g. using either Windows
> > > > or Linux for playback) the recording on the target results in a
> > > > captured file with missing samples and audible pops/clicks. In this
> > > > scenario the UAC2 gadget is configured with c_sync == asynchronous. I
> > > > wouldn't expect things to improve with c_sync == adaptive since you
> > > > mentioned in your patch that it always reports back the nominal
> > > > frequency to the host from the feedback endpoint.
> > > >
> > > > Do you have any suggestions that might explain (the above) behavior.
> > > > Can you describe your test environment in more detail so that I can
> > > > perhaps re-create it? What Linux target are you using with your tests?
> > > > You mentioned you tested an 8x8 playback/capture scenario. Can you
> > > > provide any details of how you performed this test and the method you
> > > > used to confirm the audio quality for the capture/playback?
> > > >
> > > > Thanks for any insights you might be able to offer . . .
> > > >
> > > > Glenn
> > >
> > > Hi Ruslan -
> > >
> > > This is a follow-up from my post yesterday. I recompiled my kernel
> > > *WITHOUT* your UAC2 patches and repeated Scenario #2 where the Linux
> > > PC plays a single channel tone to the BeagleBone Black where it's
> > > recorded with 'arecord'. Yesterday, I recorded garbled audio on the
> > > target but today, without any UAC2 kernel patches, the recorded audio
> > > on the target is glitch-free and appears to be bit-perfect.
> > >
> > > This experiment leads me to believe your patches may be inadvertently
> > > corrupting the data-path. Have you been able to repeat my experiment
> > > and either confirm or refute my findings? I am interested to learn
> > > more how you tested your patches and whether it's something I can
> > > recreate here.
> > >
> > > Assuming we can sort out this data corruption issue, what are your
> > > thoughts on how the Linux target device can properly provide the
> > > Windows feedback endpoint with real frequency updates rather than the
> > > constant nominal frequency. If I understood your patch notes correctly
> > > it seems this is an outstanding issue that requires additional
> > > attention. I'm a bit of a noob when it comes to how this might be
> > > addressed.
> > >
> > > Thanks for your continued insights and support . . .
> > >
> > > Glenn
> >
> > Hi Glenn & Ruslan,
> >
> > Do you know why WIN10 can't recognized UAC2 device if I configure the
> > sample rate as 48000HZ? Configuring sample rate as 44100HZ, the playback
> > function would work well at my platforms (chipidea IP), no glitch is
> > heard. At WIN10, I use Windows Media Player, at board side I use command:
>
> That's known issue, Windows is more strict with UAC2 descriptors, try to apply
> my patch "usb: gadget: f_uac2: always increase endpoint max_packet_size
> by one audio slot" that I shared in this email:
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.spinics.net%2Flists%2Flinux-usb%2Fmsg205077.html&data=04%7C01%7Cpeter.chen%40nxp.com%7C1c037c406a47423725e008d89d0b77c1%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637432019865845335%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=7jE6EEi4ZifLMVNgs3%2FygUkBjCrcIrtvIjX2%2FVwbwUM%3D&reserved=0
>
Better than before, the Device Manager could recognize this sound device,
But Windows Media Player can't play it, see attached.
Meanwhile, I find your proposal patch [1] is not based on your UAC2 patch which
changed bmAttributes as USB_ENDPOINT_XFER_ISOC only at descriptor.
[1] https://www.spinics.net/lists/linux-usb/msg205077.html
--
Thanks,
Peter Chen
[-- Attachment #2: wmp_can't_play.PNG --]
[-- Type: image/png, Size: 6717 bytes --]
^ permalink raw reply [flat|nested] 31+ messages in thread
end of thread, other threads:[~2020-12-11 7:24 UTC | newest]
Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-08 0:18 [PATCH 0/3] UAC2 Gadget: feedback endpoint support Ruslan Bilovol
2020-11-08 0:18 ` [PATCH 1/3] usb: gadget: f_uac2/u_audio: add " Ruslan Bilovol
2020-11-11 9:26 ` Peter Chen
2020-11-12 22:41 ` Ruslan Bilovol
2020-11-08 0:18 ` [PATCH 2/3] usb: gadget: f_uac2: add adaptive sync support for capture Ruslan Bilovol
2020-11-11 9:18 ` Peter Chen
2020-11-12 22:39 ` Ruslan Bilovol
2020-11-26 11:13 ` Jerome Brunet
2020-12-04 14:03 ` Ruslan Bilovol
2020-11-08 0:18 ` [PATCH 3/3] usb: gadget: u_audio: add real feedback implementation Ruslan Bilovol
2020-11-09 8:24 ` Pavel Hofman
2020-11-09 8:25 ` Pavel Hofman
2020-11-12 11:26 ` Pavel Hofman
2020-11-11 9:30 ` [PATCH 0/3] UAC2 Gadget: feedback endpoint support Peter Chen
2020-11-12 23:20 ` Ruslan Bilovol
2020-11-13 15:35 ` Glenn Schmottlach
2020-11-22 19:51 ` Ruslan Bilovol
2020-11-25 19:28 ` Glenn Schmottlach
2020-11-28 23:26 ` Ruslan Bilovol
2020-12-01 21:43 ` Glenn Schmottlach
2020-12-02 22:04 ` Glenn Schmottlach
2020-12-03 10:09 ` Peter Chen
2020-12-03 22:07 ` Glenn Schmottlach
2020-12-10 12:59 ` Ruslan Bilovol
2020-12-11 7:22 ` Peter Chen
2020-12-10 12:46 ` Ruslan Bilovol
2020-11-26 13:16 ` Jerome Brunet
2020-11-26 13:44 ` Pavel Hofman
2020-12-04 14:39 ` Ruslan Bilovol
2020-12-04 18:08 ` Pavel Hofman
2020-12-04 14:35 ` Ruslan Bilovol
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.