hi,
The function xhci_event_xfer_isoc() of sys/dev/usb/xhci.c at line 954
does not work with zero length multi-TRB inbound transfer.
949 /*
950 * If we queued two TRBs for a frame and this is the second TRB,
951 * check if the first TRB needs accounting since it might not
have
952 * raised an interrupt in case of full data received.
953 */
954 if ((letoh32(xp->ring.trbs[trb_idx].trb_flags) &
XHCI_TRB_TYPE_MASK) ==
955 XHCI_TRB_TYPE_NORMAL) {
956 frame_idx--;
957 if (trb_idx == 0)
958 trb0_idx = xp->ring.ntrb - 2;
959 else
960 trb0_idx = trb_idx - 1;
961 if (xfer->frlengths[frame_idx] == 0) {
962 xfer->frlengths[frame_idx] =
XHCI_TRB_LEN(letoh32(
963 xp->ring.trbs[trb0_idx].trb_status));
964 }
965 }
966
967 xfer->frlengths[frame_idx] +=
968 XHCI_TRB_LEN(letoh32(xp->ring.trbs[trb_idx].trb_status)) -
remain;
969 xfer->actlen += xfer->frlengths[frame_idx];
When a multi-TRB inbound transfer TD completes with transfer length = 0,
the HC should generate two events: 1st event for ISOCH TRB /w ISP|CHAIN
and 2nd event for NORMAL TRB w/ ISP|IOC.
Transfer Length field (it's remain length, actually) of each event is
same as requested length is, i.e., transferred length is 0.
So when the first event raises the frlengths is set to 0 at line 967.
It's correct.
On second event, as the comment describes, xhci.c tries to calculate
the 1st TRB xfer length at lines 954-965. The requested length of
1st TRB is stored into frlengths -- even though the xfer len is 0.
If frlengths = 0, we cannot distinguish the case the first event is
not raised from the case the transferred length is 0.
The frlengths is already 0 so the requested length of 1st TRB is stored.
For example, I applied debug printf [*1], and run
mplayer tv:// for my webcam.
I see...
#25 remain 1024 type 5 origlen 1024 frlengths[25] 0
#25 (omitted) frlen[25] 1024
#26 remain 2048 type 1 origlen 2048 frlengths[25] 1024
These console logs show a 3072 bytes frame is splitted into
two TRBs and got 0 bytes. The first TRB transfers 0 bytes and
the second TRB transfers 0, too, but it results 1024 bytes.
My proposal patch [*2] adds a flag to xhci_xfer that indicates the
TRB processed by xhci.c previously has CHAIN bit, and updates the
frlengths only when that flag is not set.
[*1]
debug printf.
It shows only splitted isochronous TDs.
--- sys/dev/usb/xhci.c.orig Sun Apr 5 10:12:37 2020
+++ sys/dev/usb/xhci.c Fri May 29 04:13:36 2020
@@ -961,12 +961,23 @@ xhci_event_xfer_isoc(struct usbd_xfer *xfer, struct xh
if (xfer->frlengths[frame_idx] == 0) {
xfer->frlengths[frame_idx] = XHCI_TRB_LEN(letoh32(
xp->ring.trbs[trb0_idx].trb_status));
+ printf("#%d (omitted) frlen[%d] %u\n",
+ trb0_idx, frame_idx, xfer->frlengths[frame_idx]);
}
}
xfer->frlengths[frame_idx] +=
XHCI_TRB_LEN(letoh32(xp->ring.trbs[trb_idx].trb_status)) - remain;
xfer->actlen += xfer->frlengths[frame_idx];
+ uint32_t trb_flags = letoh32(xp->ring.trbs[trb_idx].trb_flags);
+ if ((trb_flags & XHCI_TRB_CHAIN) ||
+ (trb_flags & XHCI_TRB_TYPE_MASK) == XHCI_TRB_TYPE_NORMAL) {
+ printf("#%d remain %u type %u origlen %u frlengths[%d] %hu\n",
+ trb_idx, remain,
+ XHCI_TRB_TYPE(trb_flags),
+ XHCI_TRB_LEN(le32toh(xp->ring.trbs[trb_idx].trb_status)),
+ frame_idx, xfer->frlengths[frame_idx]);
+ }
if (xx->index != trb_idx)
return (1);
[*2]
patch
--- sys/dev/usb/xhcivar.h.orig Sun Oct 6 21:19:28 2019
+++ sys/dev/usb/xhcivar.h Fri May 22 04:19:57 2020
@@ -40,6 +40,7 @@ struct xhci_xfer {
struct usbd_xfer xfer;
int index; /* Index of the last TRB */
size_t ntrb; /* Number of associated TRBs */
+ bool trb_chained;
};
struct xhci_ring {
--- sys/dev/usb/xhci.c.orig Sun Apr 5 10:12:37 2020
+++ sys/dev/usb/xhci.c Thu Jun 4 05:39:03 2020
@@ -958,15 +958,23 @@ xhci_event_xfer_isoc(struct usbd_xfer *xfer, struct xh
trb0_idx = xp->ring.ntrb - 2;
else
trb0_idx = trb_idx - 1;
- if (xfer->frlengths[frame_idx] == 0) {
+ if (xfer->frlengths[frame_idx] == 0 && !xx->trb_chained) {
xfer->frlengths[frame_idx] = XHCI_TRB_LEN(letoh32(
xp->ring.trbs[trb0_idx].trb_status));
}
}
- xfer->frlengths[frame_idx] +=
- XHCI_TRB_LEN(letoh32(xp->ring.trbs[trb_idx].trb_status)) - remain;
- xfer->actlen += xfer->frlengths[frame_idx];
+ if (!xx->trb_chained) {
+ xfer->frlengths[frame_idx] +=
+ XHCI_TRB_LEN(letoh32(xp->ring.trbs[trb_idx].trb_status)) -
+ remain;
+ xfer->actlen += xfer->frlengths[frame_idx];
+ }
+
+ if (letoh32(xp->ring.trbs[trb_idx].trb_flags) & XHCI_TRB_CHAIN)
+ xx->trb_chained = true;
+ else
+ xx->trb_chained = false;
if (xx->index != trb_idx)
return (1);
