On Thu, Jun 13, 2019 at 06:03:03AM +0300, Artturi Alm wrote:
> On Wed, Jun 12, 2019 at 03:55:59PM +0200, Alexandre Ratchov wrote:
> > Currenty USB device driver code has no way to obtain how many frames
> > can be scheduled on the HC. If it attempts to schedule too many
> > frames, usbd_transfer() fails or silently misbehaves.
> >
> > For audio this is a big problem because the max frames count
> > determines the block size which must be reported to upper layers
> > before any transfer starts. This makes impossible to implement uaudio
> > properly. Currently there's a temporary hack to workaround this, which
> > limits the block size. Now that uaudio is in and works, it's time to
> > start removing such hacks.
> >
> > Similarly, driver code needs to know the minimum number of frames per
> > transfer to get an interrupt (ie the completion callback). Indeed, if
> > the transfer frame count is not rounded to it, we silently miss the
> > interrupt, the completion call-back is not called and playback
> > stutters.
> >
> > The diff below adds a new usbd_bus_info() function which reports the
> > maximum frames that can be scheduled and the minimum frames per
> > transfer to get an interrupt.
> >
> > [snip]
> > Index: usbdi.c
> > ===================================================================
> > RCS file: /cvs/src/sys/dev/usb/usbdi.c,v
> > retrieving revision 1.100
> > diff -u -p -u -p -r1.100 usbdi.c
> > --- usbdi.c 18 Nov 2018 16:33:26 -0000 1.100
> > +++ usbdi.c 12 Jun 2019 13:33:41 -0000
> > @@ -275,6 +275,12 @@ usbd_close_pipe(struct usbd_pipe *pipe)
> > return (USBD_NORMAL_COMPLETION);
> > }
> >
> > +void
> > +usbd_bus_info(struct usbd_device *dev, struct usbd_bus_info *info)
> > +{
> > + dev->bus->methods->info(dev, info);
> > +}
> > +
> > usbd_status
> > usbd_transfer(struct usbd_xfer *xfer)
> > {
> > [snip]
>
> i think above is not good enough with only ehci, ohci, uhci and xhci
> being taken care of in [snip]'ed parts of the diff unfortunately:]
> just thinking about dwc2.
>
Thanks. I grep'ed for usbd_bus_methods, and dwc2 seems to be the only
one I missed. Below is a new diff with dwc2 bits and few fixes. I've
no arm/octeon machines to test, anyone has one for a quick test?
Interestingly, dwc2 supports at most DWC2_MAXISOCPACKETS frames, which
is defined to 16 only. Without this diff, the uaudio driver would
attempt to schedule ~48 frames which would trigger a KASSERT() in
dwc2_device_start() immediately.
BTW, 16 is too small for usb2.0 devices to work at all. I'd recommend
512, which is two blocks of 20ms each.
Index: ehci.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/ehci.c,v
retrieving revision 1.204
diff -u -p -r1.204 ehci.c
--- ehci.c 31 Mar 2019 06:16:38 -0000 1.204
+++ ehci.c 13 Jun 2019 07:25:31 -0000
@@ -113,6 +113,7 @@ struct ehci_pipe {
u_int8_t ehci_reverse_bits(u_int8_t, int);
usbd_status ehci_open(struct usbd_pipe *);
+void ehci_info(struct usbd_device *, struct usbd_bus_info *);
int ehci_setaddr(struct usbd_device *, int);
void ehci_poll(struct usbd_bus *);
void ehci_softintr(void *);
@@ -225,6 +226,7 @@ struct usbd_bus_methods ehci_bus_methods
.do_poll = ehci_poll,
.allocx = ehci_allocx,
.freex = ehci_freex,
+ .info = ehci_info,
};
struct usbd_pipe_methods ehci_root_ctrl_methods = {
@@ -491,6 +493,21 @@ ehci_init(struct ehci_softc *sc)
sc->sc_flsize * sizeof(struct ehci_soft_itd *));
usb_freemem(&sc->sc_bus, &sc->sc_fldma);
return (err);
+}
+
+void
+ehci_info(struct usbd_device *dev, struct usbd_bus_info *info)
+{
+ /*
+ * XXX: even if most hosts have 1024 frame lists, only 256
+ * frame transfers work reliably. As soon as 256 is exceeded,
+ * we start getting zeroed frames if multiple device are
+ * running simultaneously. Set this to sc->sc_fl_size, once
+ * ehci is fixed. Interrups occur every 1m, despite the
+ * EHCI_CMD_ITC_2 setting.
+ */
+ info->nframes_max = 256;
+ info->intr_thres = (dev->speed >= USB_SPEED_HIGH) ? 8 : 1;
}
int
Index: ohci.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/ohci.c,v
retrieving revision 1.156
diff -u -p -r1.156 ohci.c
--- ohci.c 11 Mar 2019 17:50:08 -0000 1.156
+++ ohci.c 13 Jun 2019 07:25:32 -0000
@@ -87,6 +87,7 @@ usbd_status ohci_alloc_std_chain(struct
struct ohci_soft_td **);
usbd_status ohci_open(struct usbd_pipe *);
+void ohci_info(struct usbd_device *, struct usbd_bus_info *);
int ohci_setaddr(struct usbd_device *, int);
void ohci_poll(struct usbd_bus *);
void ohci_softintr(void *);
@@ -236,6 +237,7 @@ struct usbd_bus_methods ohci_bus_methods
.do_poll = ohci_poll,
.allocx = ohci_allocx,
.freex = ohci_freex,
+ .info = ohci_info,
};
struct usbd_pipe_methods ohci_root_ctrl_methods = {
@@ -931,6 +933,13 @@ ohci_init(struct ohci_softc *sc)
bad1:
usb_freemem(&sc->sc_bus, &sc->sc_hccadma);
return (err);
+}
+
+void
+ohci_info(struct usbd_device *dev, struct usbd_bus_info *info)
+{
+ info->nframes_max = OHCI_SITD_CHUNK;
+ info->intr_thres = 1;
}
struct usbd_xfer *
Index: uaudio.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/uaudio.c,v
retrieving revision 1.144
diff -u -p -r1.144 uaudio.c
--- uaudio.c 9 May 2019 07:09:04 -0000 1.144
+++ uaudio.c 13 Jun 2019 07:25:33 -0000
@@ -382,6 +382,7 @@ struct uaudio_softc {
unsigned int ufps; /* USB frames per second */
unsigned int sync_pktsz; /* size of sync packet */
unsigned int host_nframes; /* max frames we can schedule */
+ unsigned int host_intr_thres; /* min frames to get an interrupt */
int diff_nsamp; /* samples play is ahead of rec */
int diff_nframes; /* frames play is ahead of rec */
@@ -2820,12 +2821,15 @@ uaudio_stream_open(struct uaudio_softc *
s->data_pipe = NULL;
s->sync_pipe = NULL;
- s->nframes_mask = 0;
- i = a->fps;
- while (i > 1000) {
- s->nframes_mask = (s->nframes_mask << 1) | 1;
- i >>= 1;
- }
+ /*
+ * Find the smallest transfer size (power of two), larger than
+ * the poll interval and the interrupt threshold.
+ */
+ i = 0;
+ while (a->fps < (sc->ufps >> i) || sc->host_intr_thres > (1 << i))
+ i++;
+
+ s->nframes_mask = (1 << i) - 1;
/* bytes per audio frame */
bpa = a->bps * a->nch;
@@ -2900,12 +2904,10 @@ uaudio_stream_open(struct uaudio_softc *
}
/*
- * Require at least 2ms block size to ensure no
- * transfer exceeds two blocks.
- *
- * XXX: use s->nframes_mask instead of 1000
+ * Require at least twice the smallest transfer size allowed
+ * as block size to ensure no transfer exceeds two blocks.
*/
- if (1000 * blksz < 2 * sc->rate * bpa) {
+ if (sc->ufps / (s->nframes_mask + 1) * blksz < 2 * sc->rate * bpa) {
printf("%s: audio block too small\n", DEVNAME(sc));
return EIO;
}
@@ -3694,6 +3696,7 @@ uaudio_attach(struct device *parent, str
struct uaudio_softc *sc = (struct uaudio_softc *)self;
struct usb_attach_arg *arg = aux;
struct usb_config_descriptor *cdesc;
+ struct usbd_bus_info info;
struct uaudio_blob desc;
/*
@@ -3720,31 +3723,29 @@ uaudio_attach(struct device *parent, str
sc->trigger_mode = 0;
sc->copy_todo = 0;
- /*
- * Ideally the USB host controller should expose the number of
- * frames we're allowed to schedule, but there's no such
- * interface. The uhci(4) driver can buffer up to 128 frames
- * (or it crashes), ehci(4) starts recording null frames if we
- * exceed 256 (micro-)frames, ohci(4) works with at most 50
- * frames.
- */
switch (sc->udev->speed) {
case USB_SPEED_LOW:
case USB_SPEED_FULL:
sc->ufps = 1000;
sc->sync_pktsz = 3;
- sc->host_nframes = 50;
break;
case USB_SPEED_HIGH:
case USB_SPEED_SUPER:
sc->ufps = 8000;
sc->sync_pktsz = 4;
- sc->host_nframes = 240;
break;
default:
printf("%s: unsupported bus speed\n", DEVNAME(sc));
return;
}
+
+ /*
+ * get the max number of frames the host supports, needed to
+ * determine the max block size
+ */
+ usbd_bus_info(sc->udev, &info);
+ sc->host_nframes = info.nframes_max;
+ sc->host_intr_thres = info.intr_thres;
if (!uaudio_process_conf(sc, &desc))
return;
Index: uhci.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/uhci.c,v
retrieving revision 1.147
diff -u -p -r1.147 uhci.c
--- uhci.c 12 Mar 2019 08:13:50 -0000 1.147
+++ uhci.c 13 Jun 2019 07:25:34 -0000
@@ -178,6 +178,7 @@ void uhci_root_intr_close(struct usbd_p
void uhci_root_intr_done(struct usbd_xfer *);
usbd_status uhci_open(struct usbd_pipe *);
+void uhci_info(struct usbd_device *, struct usbd_bus_info *);
void uhci_poll(struct usbd_bus *);
void uhci_softintr(void *);
@@ -253,6 +254,7 @@ struct usbd_bus_methods uhci_bus_methods
.do_poll = uhci_poll,
.allocx = uhci_allocx,
.freex = uhci_freex,
+ .info = uhci_info,
};
struct usbd_pipe_methods uhci_root_ctrl_methods = {
@@ -486,6 +488,13 @@ uhci_init(struct uhci_softc *sc)
UHCI_INTR_IOCE | UHCI_INTR_SPIE); /* enable interrupts */
return (uhci_run(sc, 1)); /* and here we go... */
+}
+
+void
+uhci_info(struct usbd_device *dev, struct usbd_bus_info *info)
+{
+ info->nframes_max = UHCI_VFRAMELIST_COUNT;
+ info->intr_thres = 1;
}
int
Index: usbdi.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/usbdi.c,v
retrieving revision 1.100
diff -u -p -r1.100 usbdi.c
--- usbdi.c 18 Nov 2018 16:33:26 -0000 1.100
+++ usbdi.c 13 Jun 2019 07:25:34 -0000
@@ -275,6 +275,12 @@ usbd_close_pipe(struct usbd_pipe *pipe)
return (USBD_NORMAL_COMPLETION);
}
+void
+usbd_bus_info(struct usbd_device *dev, struct usbd_bus_info *info)
+{
+ dev->bus->methods->info(dev, info);
+}
+
usbd_status
usbd_transfer(struct usbd_xfer *xfer)
{
Index: usbdi.h
===================================================================
RCS file: /cvs/src/sys/dev/usb/usbdi.h,v
retrieving revision 1.70
diff -u -p -r1.70 usbdi.h
--- usbdi.h 1 May 2018 18:14:46 -0000 1.70
+++ usbdi.h 13 Jun 2019 07:25:35 -0000
@@ -86,9 +86,15 @@ typedef void (*usbd_callback)(struct usb
#define DEVINFOSIZE 1024
+struct usbd_bus_info {
+ int nframes_max; /* max frames we can schedule */
+ int intr_thres; /* interrupt threshold in (micro-)frames */
+};
+
usbd_status usbd_open_pipe(struct usbd_interface *iface, u_int8_t address,
u_int8_t flags, struct usbd_pipe **pipe);
usbd_status usbd_close_pipe(struct usbd_pipe *pipe);
+void usbd_bus_info(struct usbd_device *dev, struct usbd_bus_info *info);
usbd_status usbd_transfer(struct usbd_xfer *req);
struct usbd_xfer *usbd_alloc_xfer(struct usbd_device *);
void usbd_free_xfer(struct usbd_xfer *xfer);
Index: usbdivar.h
===================================================================
RCS file: /cvs/src/sys/dev/usb/usbdivar.h,v
retrieving revision 1.79
diff -u -p -r1.79 usbdivar.h
--- usbdivar.h 27 Nov 2018 14:56:09 -0000 1.79
+++ usbdivar.h 13 Jun 2019 07:25:35 -0000
@@ -65,6 +65,7 @@ struct usbd_bus_methods {
void (*do_poll)(struct usbd_bus *);
struct usbd_xfer * (*allocx)(struct usbd_bus *);
void (*freex)(struct usbd_bus *, struct usbd_xfer *);
+ void (*info)(struct usbd_device *, struct
usbd_bus_info *);
};
struct usbd_pipe_methods {
Index: xhci.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/xhci.c,v
retrieving revision 1.104
diff -u -p -r1.104 xhci.c
--- xhci.c 21 May 2019 09:20:40 -0000 1.104
+++ xhci.c 13 Jun 2019 07:25:35 -0000
@@ -126,6 +126,7 @@ void xhci_timeout_task(void *);
/* USBD Bus Interface. */
usbd_status xhci_pipe_open(struct usbd_pipe *);
+void xhci_info(struct usbd_device *, struct usbd_bus_info *);
int xhci_setaddr(struct usbd_device *, int);
void xhci_softintr(void *);
void xhci_poll(struct usbd_bus *);
@@ -161,6 +162,7 @@ struct usbd_bus_methods xhci_bus_methods
.do_poll = xhci_poll,
.allocx = xhci_allocx,
.freex = xhci_freex,
+ .info = xhci_info,
};
struct usbd_pipe_methods xhci_root_ctrl_methods = {
@@ -466,6 +468,18 @@ xhci_config(struct xhci_softc *sc)
DPRINTF(("%s: USBCMD=%#x\n", DEVNAME(sc), XOREAD4(sc, XHCI_USBCMD)));
DPRINTF(("%s: IMAN=%#x\n", DEVNAME(sc), XRREAD4(sc, XHCI_IMAN(0))));
+}
+
+void
+xhci_info(struct usbd_device *dev, struct usbd_bus_info *info)
+{
+ info->nframes_max = XHCI_MAX_XFER;
+
+ /*
+ * We get only one interrupt per frame, ie 1 interrupt every 8
+ * microframes
+ */
+ info->intr_thres = (dev->speed >= USB_SPEED_HIGH) ? 8 : 1;
}
int
Index: dwc2/dwc2.c
===================================================================
RCS file: /cvs/src/sys/dev/usb/dwc2/dwc2.c,v
retrieving revision 1.48
diff -u -p -r1.48 dwc2.c
--- dwc2/dwc2.c 14 Mar 2019 04:28:10 -0000 1.48
+++ dwc2/dwc2.c 13 Jun 2019 07:25:36 -0000
@@ -89,6 +89,7 @@ STATIC usbd_status dwc2_open(struct usbd
STATIC int dwc2_setaddr(struct usbd_device *, int);
STATIC void dwc2_poll(struct usbd_bus *);
STATIC void dwc2_softintr(void *);
+STATIC void dwc2_info(struct usbd_device *, struct usbd_bus_info *);
#if 0
STATIC usbd_status dwc2_allocm(struct usbd_bus *, struct usb_dma *,
uint32_t);
@@ -179,6 +180,7 @@ STATIC struct usbd_bus_methods dwc2_bus_
#if 0
.get_lock = dwc2_get_lock,
#endif
+ .info = dwc2_info,
};
STATIC struct usbd_pipe_methods dwc2_root_ctrl_methods = {
@@ -610,6 +612,13 @@ dwc2_device_clear_toggle(struct usbd_pip
{
DPRINTF("toggle %d -> 0", pipe->endpoint->savedtoggle);
+}
+
+STATIC void
+dwc2_info(struct usbd_device *dev, struct usbd_bus_info *info)
+{
+ info->nframes_max = DWC2_MAXISOCPACKETS;
+ info->intr_thres = (dev->speed >= USB_SPEED_HIGH) ? 8 : 1;
}
/***********************************************************************/
