hrev52975 adds 2 changesets to branch 'master'
old head: 9ba45ef9246e58666c3f8e296d5dcd8a3b64d15d
new head: 9bdd88ebe3c87dcbffdd415a060903e95cd4a55b
overview:
https://git.haiku-os.org/haiku/log/?qt=range&q=9bdd88ebe3c8+%5E9ba45ef9246e
----------------------------------------------------------------------------
187a2c6ce705: XHCI: Completely overhaul endpoint configuration logic.
The code as it was before was very hard to follow and incorrect in
a number of corner cases, as well as not being very clear about what
the TODOs were.
It now follows the spec much more closely (especially in interval
computation) and contains more details on where it is still lacking.
This probably does not fix much (if anything) as is, but it paves the
way for future isochronous support.
9bdd88ebe3c8: XHCI: Granularize locking.
This is not so important now (though it is a mild performance improvement
when running transfers during commands), but it will be when the debug
transfer hooks are implemented, as we will need to use these to determine
if it is safe to queue and poll for transfers or not.
[ Augustin Cavalier <waddlesplash@xxxxxxxxx> ]
----------------------------------------------------------------------------
2 files changed, 161 insertions(+), 122 deletions(-)
src/add-ons/kernel/busses/usb/xhci.cpp | 264 ++++++++++++++++-------------
src/add-ons/kernel/busses/usb/xhci.h | 19 ++-
############################################################################
Commit: 187a2c6ce705ef333ff2831b43be5b6047d0b44f
URL: https://git.haiku-os.org/haiku/commit/?id=187a2c6ce705
Author: Augustin Cavalier <waddlesplash@xxxxxxxxx>
Date: Fri Mar 8 22:09:55 2019 UTC
XHCI: Completely overhaul endpoint configuration logic.
The code as it was before was very hard to follow and incorrect in
a number of corner cases, as well as not being very clear about what
the TODOs were.
It now follows the spec much more closely (especially in interval
computation) and contains more details on where it is still lacking.
This probably does not fix much (if anything) as is, but it paves the
way for future isochronous support.
----------------------------------------------------------------------------
diff --git a/src/add-ons/kernel/busses/usb/xhci.cpp
b/src/add-ons/kernel/busses/usb/xhci.cpp
index c37a839492..c7d7ebc455 100644
--- a/src/add-ons/kernel/busses/usb/xhci.cpp
+++ b/src/add-ons/kernel/busses/usb/xhci.cpp
@@ -1234,9 +1234,10 @@ XHCI::AllocateDevice(Hub *parent, int8 hubAddress, uint8
hubPort,
break;
}
- // configure the Control endpoint 0 (type 4)
- if (ConfigureEndpoint(slot, 0, 4, device->trb_addr, 0,
- maxPacketSize, maxPacketSize & 0x7ff, speed) != B_OK) {
+ // configure the Control endpoint 0
+ if (ConfigureEndpoint(slot, 0, USB_OBJECT_CONTROL_PIPE, false,
+ device->trb_addr, 0, maxPacketSize, maxPacketSize &
0x7ff,
+ speed) != B_OK) {
TRACE_ERROR("unable to configure default control endpoint\n");
delete_area(device->input_ctx_area);
delete_area(device->device_ctx_area);
@@ -1479,22 +1480,12 @@ XHCI::_InsertEndpointForPipe(Pipe *pipe)
uint8 endpoint = id + 1;
- // configure the Control endpoint 0 (type 4)
- uint32 type = 4;
- if ((pipe->Type() & USB_OBJECT_INTERRUPT_PIPE) != 0)
- type = 3;
- if ((pipe->Type() & USB_OBJECT_BULK_PIPE) != 0)
- type = 2;
- if ((pipe->Type() & USB_OBJECT_ISO_PIPE) != 0)
- type = 1;
- type |= (pipe->Direction() == Pipe::In) ? (1 << 2) : 0;
-
TRACE("trb_addr 0x%" B_PRIxPHYSADDR "\n",
device->endpoints[id].trb_addr);
- status_t status = ConfigureEndpoint(device->slot, id, type,
- device->endpoints[id].trb_addr, pipe->Interval(),
- pipe->MaxPacketSize(), pipe->MaxPacketSize() & 0x7ff,
- usbDevice->Speed());
+ status_t status = ConfigureEndpoint(device->slot, id,
pipe->Type(),
+ pipe->Direction() == Pipe::In,
device->endpoints[id].trb_addr,
+ pipe->Interval(), pipe->MaxPacketSize(),
+ pipe->MaxPacketSize() & 0x7ff, usbDevice->Speed());
if (status != B_OK) {
TRACE_ERROR("unable to configure endpoint\n");
return status;
@@ -1629,8 +1620,9 @@ XHCI::_UnlinkDescriptorForPipe(xhci_td *descriptor,
xhci_endpoint *endpoint)
status_t
-XHCI::ConfigureEndpoint(uint8 slot, uint8 number, uint8 type, uint64 ringAddr,
- uint16 interval, uint16 maxPacketSize, uint16 maxFrameSize, usb_speed
speed)
+XHCI::ConfigureEndpoint(uint8 slot, uint8 number, uint8 type, bool directionIn,
+ uint64 ringAddr, uint16 interval, uint16 maxPacketSize, uint16
maxFrameSize,
+ usb_speed speed)
{
struct xhci_device* device = &fDevices[slot];
@@ -1639,65 +1631,91 @@ XHCI::ConfigureEndpoint(uint8 slot, uint8 number, uint8
type, uint64 ringAddr,
uint64 qwendpoint2 = 0;
uint32 dwendpoint4 = 0;
- // Compute interval
- uint16 calcInterval = 0;
- if (speed == USB_SPEED_HIGHSPEED && (type == 4 || type == 2)) {
- if (interval != 0) {
- while ((1 << calcInterval) <= interval)
- calcInterval++;
- calcInterval--;
+ // Compute and assign the endpoint type. (XHCI 1.1 § 6.2.3 Table 6-9
p429.)
+ uint8 xhciType = 4;
+ if ((type & USB_OBJECT_INTERRUPT_PIPE) != 0)
+ xhciType = 3;
+ if ((type & USB_OBJECT_BULK_PIPE) != 0)
+ xhciType = 2;
+ if ((type & USB_OBJECT_ISO_PIPE) != 0)
+ xhciType = 1;
+ xhciType |= directionIn ? (1 << 2) : 0;
+ dwendpoint1 |= ENDPOINT_1_EPTYPE(xhciType);
+
+ // Compute and assign interval. (XHCI 1.1 § 6.2.3.6 p433.)
+ uint16 calcInterval;
+ if ((type & USB_OBJECT_BULK_PIPE) != 0
+ || (type & USB_OBJECT_CONTROL_PIPE) != 0) {
+ // Bulk and Control endpoints never issue NAKs.
+ calcInterval = 0;
+ } else {
+ switch (speed) {
+ case USB_SPEED_FULLSPEED:
+ if ((type & USB_OBJECT_ISO_PIPE) != 0) {
+ // Convert 1-16 into 3-18.
+ calcInterval = min_c(max_c(interval, 1), 16) +
2;
+ break;
+ }
+
+ // fall through
+ case USB_SPEED_LOWSPEED: {
+ // Convert 1ms-255ms into 3-10.
+
+ // Find the index of the highest set bit in "interval".
+ uint32 temp = min_c(max_c(interval, 1), 255);
+ for (calcInterval = 0; temp != 1; calcInterval++)
+ temp = temp >> 1;
+ calcInterval += 3;
+ break;
+ }
+
+ case USB_SPEED_HIGHSPEED:
+ case USB_SPEED_SUPER:
+ default:
+ // Convert 1-16 into 0-15.
+ calcInterval = min_c(max_c(interval, 1), 16) - 1;
+ break;
}
- }
- if ((type & 0x3) == 3
- && (speed == USB_SPEED_FULLSPEED || speed ==
USB_SPEED_LOWSPEED)) {
- while ((1 << calcInterval) <= interval * 8)
- calcInterval++;
- calcInterval--;
- }
- if ((type & 0x3) == 1 && speed == USB_SPEED_FULLSPEED) {
- calcInterval = interval + 2;
- }
- if (((type & 0x3) == 1 || (type & 0x3) == 3)
- && (speed == USB_SPEED_HIGHSPEED || speed ==
USB_SPEED_SUPER)) {
- calcInterval = interval - 1;
}
dwendpoint0 |= ENDPOINT_0_INTERVAL(calcInterval);
- // Assigning CERR for non-isochronous endpoints
- if ((type & 0x3) != 1)
+ // For non-isochronous endpoints, we want the controller to retry failed
+ // transfers, if possible. (XHCI 1.1 § 4.10.2.3 p189.)
+ if (!(type & USB_OBJECT_ISO_PIPE))
dwendpoint1 |= ENDPOINT_1_CERR(3);
- dwendpoint1 |= ENDPOINT_1_EPTYPE(type);
-
- // Assigning MaxBurst for HighSpeed
+ // Assign maximum burst size.
+ // TODO: While computing the maximum burst this way is correct for USB2
+ // devices, it is merely acceptable for USB3 devices, which have a more
+ // correct value stored in the Companion Descriptor. (Further, this
value
+ // in the USB3 Companion Descriptor is to be used for *all* endpoints,
not
+ // just Interrupt and Isoch ones.)
uint8 maxBurst = (maxPacketSize & 0x1800) >> 11;
- maxPacketSize = (maxPacketSize & 0x7ff);
- if (speed == USB_SPEED_HIGHSPEED &&
- ((type & 0x3) == 1 || (type & 0x3) == 3)) {
+ if (speed >= USB_SPEED_HIGHSPEED
+ && (((type & USB_OBJECT_INTERRUPT_PIPE) != 0)
+ || (type & USB_OBJECT_ISO_PIPE) != 0)) {
dwendpoint1 |= ENDPOINT_1_MAXBURST(maxBurst);
}
- // TODO Assign MaxBurst for SuperSpeed
+ // Assign maximum packet size, set the ring address, and set the
+ // "Dequeue Cycle State" bit. (XHCI 1.1 § 6.2.3 Table 6-10 p430.)
dwendpoint1 |= ENDPOINT_1_MAXPACKETSIZE(maxPacketSize);
qwendpoint2 |= ENDPOINT_2_DCS_BIT | ringAddr;
- // Assign MaxESITPayload
- // Assign AvgTRBLength
- switch (type) {
- case 4:
- dwendpoint4 = ENDPOINT_4_AVGTRBLENGTH(8);
- break;
- case 1:
- case 3:
- case 5:
- case 7:
- dwendpoint4 = ENDPOINT_4_AVGTRBLENGTH(maxPacketSize * 4)
- | ENDPOINT_4_MAXESITPAYLOAD((
- (maxBurst+1) * maxPacketSize));
- break;
- default:
- dwendpoint4 = ENDPOINT_4_AVGTRBLENGTH(maxPacketSize *
4);
- break;
+ // Assign average TRB length.
+ if ((type & USB_OBJECT_CONTROL_PIPE) != 0) {
+ // Control pipes are a special case, as they rarely have
+ // outbound transfers of any substantial size.
+ dwendpoint4 |= ENDPOINT_4_AVGTRBLENGTH(8);
+ } else {
+ dwendpoint4 |= ENDPOINT_4_AVGTRBLENGTH(maxPacketSize * 4);
+ }
+
+ // Assign maximum ESIT payload. (XHCI 1.1 § 4.14.2 p250.)
+ // TODO: This computation is *only* correct for USB2 devices.
+ if (((type & USB_OBJECT_INTERRUPT_PIPE) != 0)
+ || ((type & USB_OBJECT_ISO_PIPE) != 0)) {
+ dwendpoint4 |= ENDPOINT_4_MAXESITPAYLOAD((maxBurst + 1) *
maxPacketSize);
}
_WriteContext(&device->input_ctx->endpoints[number].dwendpoint0,
diff --git a/src/add-ons/kernel/busses/usb/xhci.h
b/src/add-ons/kernel/busses/usb/xhci.h
index e109b59c24..653059711f 100644
--- a/src/add-ons/kernel/busses/usb/xhci.h
+++ b/src/add-ons/kernel/busses/usb/xhci.h
@@ -130,9 +130,9 @@ private:
// Endpoint management
status_t ConfigureEndpoint(uint8
slot, uint8 number,
- uint8
type, uint64 ringAddr,
- uint16
interval, uint16 maxPacketSize,
- uint16
maxFrameSize, usb_speed speed);
+ uint8 type, bool directionIn,
uint64 ringAddr,
+ uint16 interval, uint16
maxPacketSize,
+ uint16 maxFrameSize, usb_speed
speed);
status_t
_InsertEndpointForPipe(Pipe *pipe);
status_t
_RemoveEndpointForPipe(Pipe *pipe);
############################################################################
Revision: hrev52975
Commit: 9bdd88ebe3c87dcbffdd415a060903e95cd4a55b
URL: https://git.haiku-os.org/haiku/commit/?id=9bdd88ebe3c8
Author: Augustin Cavalier <waddlesplash@xxxxxxxxx>
Date: Fri Mar 8 22:40:21 2019 UTC
XHCI: Granularize locking.
This is not so important now (though it is a mild performance improvement
when running transfers during commands), but it will be when the debug
transfer hooks are implemented, as we will need to use these to determine
if it is safe to queue and poll for transfers or not.
----------------------------------------------------------------------------
diff --git a/src/add-ons/kernel/busses/usb/xhci.cpp
b/src/add-ons/kernel/busses/usb/xhci.cpp
index c7d7ebc455..dd461b7413 100644
--- a/src/add-ons/kernel/busses/usb/xhci.cpp
+++ b/src/add-ons/kernel/busses/usb/xhci.cpp
@@ -189,16 +189,16 @@ XHCI::XHCI(pci_info *info, Stack *stack)
fErstArea(-1),
fDcbaArea(-1),
fCmdCompSem(-1),
- fFinishTransfersSem(-1),
- fFinishThread(-1),
fStopThreads(false),
- fFinishedHead(NULL),
fRootHub(NULL),
fRootHubAddress(0),
fPortCount(0),
fSlotCount(0),
fScratchpadCount(0),
fContextSizeShift(0),
+ fFinishedHead(NULL),
+ fFinishTransfersSem(-1),
+ fFinishThread(-1),
fEventSem(-1),
fEventThread(-1),
fEventIdx(0),
@@ -207,6 +207,8 @@ XHCI::XHCI(pci_info *info, Stack *stack)
fCmdCcs(1)
{
B_INITIALIZE_SPINLOCK(&fSpinlock);
+ mutex_init(&fFinishedLock, "XHCI finished transfers");
+ mutex_init(&fEventLock, "XHCI event handler");
if (BusManager::InitCheck() < B_OK) {
TRACE_ERROR("bus manager failed to init\n");
@@ -402,6 +404,9 @@ XHCI::~XHCI()
wait_for_thread(fFinishThread, &result);
wait_for_thread(fEventThread, &result);
+ mutex_destroy(&fFinishedLock);
+ mutex_destroy(&fEventLock);
+
remove_io_interrupt_handler(fIRQ, InterruptHandler, (void *)this);
delete_area(fRegisterArea);
@@ -2114,11 +2119,13 @@ XHCI::HandleTransferComplete(xhci_trb* trb)
td->trb_completion_code = completionCode;
td->trb_left = remainder;
+
// add descriptor to finished list
- Lock();
+ mutex_lock(&fFinishedLock);
td->next = fFinishedHead;
fFinishedHead = td;
- Unlock();
+ mutex_unlock(&fFinishedLock);
+
release_sem(fFinishTransfersSem);
TRACE("HandleTransferComplete td %p done\n", td);
} else {
@@ -2361,52 +2368,61 @@ XHCI::CompleteEvents()
if (semCount > 0)
acquire_sem_etc(fEventSem, semCount,
B_RELATIVE_TIMEOUT, 0);
- uint16 i = fEventIdx;
- uint8 j = fEventCcs;
- uint8 t = 2;
-
- while (1) {
- uint32 temp =
B_LENDIAN_TO_HOST_INT32(fEventRing[i].dwtrb3);
- uint8 event = TRB_3_TYPE_GET(temp);
- TRACE("event[%u] = %u (0x%016" B_PRIx64 " 0x%08"
B_PRIx32 " 0x%08"
- B_PRIx32 ")\n", i, event, fEventRing[i].qwtrb0,
- fEventRing[i].dwtrb2,
B_LENDIAN_TO_HOST_INT32(fEventRing[i].dwtrb3));
- uint8 k = (temp & TRB_3_CYCLE_BIT) ? 1 : 0;
- if (j != k)
- break;
+ ProcessEvents();
+ }
+}
- switch (event) {
- case TRB_TYPE_COMMAND_COMPLETION:
- HandleCmdComplete(&fEventRing[i]);
- break;
- case TRB_TYPE_TRANSFER:
- HandleTransferComplete(&fEventRing[i]);
- break;
- case TRB_TYPE_PORT_STATUS_CHANGE:
- TRACE("port change detected\n");
- break;
- default:
- TRACE_ERROR("Unhandled event = %u\n", event);
- break;
- }
- i++;
- if (i == XHCI_MAX_EVENTS) {
- i = 0;
- j ^= 1;
- if (!--t)
- break;
- }
- }
+void
+XHCI::ProcessEvents()
+{
+ MutexLocker _(fEventLock);
+
+ uint16 i = fEventIdx;
+ uint8 j = fEventCcs;
+ uint8 t = 2;
+
+ while (1) {
+ uint32 temp = B_LENDIAN_TO_HOST_INT32(fEventRing[i].dwtrb3);
+ uint8 event = TRB_3_TYPE_GET(temp);
+ TRACE("event[%u] = %u (0x%016" B_PRIx64 " 0x%08" B_PRIx32 "
0x%08"
+ B_PRIx32 ")\n", i, event, fEventRing[i].qwtrb0,
+ fEventRing[i].dwtrb2,
B_LENDIAN_TO_HOST_INT32(fEventRing[i].dwtrb3));
+ uint8 k = (temp & TRB_3_CYCLE_BIT) ? 1 : 0;
+ if (j != k)
+ break;
- fEventIdx = i;
- fEventCcs = j;
+ switch (event) {
+ case TRB_TYPE_COMMAND_COMPLETION:
+ HandleCmdComplete(&fEventRing[i]);
+ break;
+ case TRB_TYPE_TRANSFER:
+ HandleTransferComplete(&fEventRing[i]);
+ break;
+ case TRB_TYPE_PORT_STATUS_CHANGE:
+ TRACE("port change detected\n");
+ break;
+ default:
+ TRACE_ERROR("Unhandled event = %u\n", event);
+ break;
+ }
- uint64 addr = fErst->rs_addr + i * sizeof(xhci_trb);
- addr |= ERST_EHB;
- WriteRunReg32(XHCI_ERDP_LO(0), (uint32)addr);
- WriteRunReg32(XHCI_ERDP_HI(0), (uint32)(addr >> 32));
+ i++;
+ if (i == XHCI_MAX_EVENTS) {
+ i = 0;
+ j ^= 1;
+ if (!--t)
+ break;
+ }
}
+
+ fEventIdx = i;
+ fEventCcs = j;
+
+ uint64 addr = fErst->rs_addr + i * sizeof(xhci_trb);
+ addr |= ERST_EHB;
+ WriteRunReg32(XHCI_ERDP_LO(0), (uint32)addr);
+ WriteRunReg32(XHCI_ERDP_HI(0), (uint32)(addr >> 32));
}
@@ -2431,13 +2447,13 @@ XHCI::FinishTransfers()
if (semCount > 0)
acquire_sem_etc(fFinishTransfersSem, semCount,
B_RELATIVE_TIMEOUT, 0);
- Lock();
+ mutex_lock(&fFinishedLock);
TRACE("finishing transfers\n");
while (fFinishedHead != NULL) {
xhci_td* td = fFinishedHead;
fFinishedHead = td->next;
td->next = NULL;
- Unlock();
+ mutex_unlock(&fFinishedLock);
TRACE("finishing transfer td %p\n", td);
@@ -2495,9 +2511,9 @@ XHCI::FinishTransfers()
transfer->Finished(callbackStatus, actualLength);
delete transfer;
FreeDescriptor(td);
- Lock();
+ mutex_lock(&fFinishedLock);
}
- Unlock();
+ mutex_unlock(&fFinishedLock);
}
}
diff --git a/src/add-ons/kernel/busses/usb/xhci.h
b/src/add-ons/kernel/busses/usb/xhci.h
index 653059711f..df9d259661 100644
--- a/src/add-ons/kernel/busses/usb/xhci.h
+++ b/src/add-ons/kernel/busses/usb/xhci.h
@@ -139,6 +139,7 @@ private:
// Event management
static int32 EventThread(void *data);
void CompleteEvents();
+ void ProcessEvents();
// Transfer management
static int32 FinishThread(void *data);
@@ -242,12 +243,8 @@ private:
spinlock fSpinlock;
sem_id fCmdCompSem;
- sem_id fFinishTransfersSem;
- thread_id fFinishThread;
bool fStopThreads;
- xhci_td * fFinishedHead;
-
// Root Hub
XHCIRootHub * fRootHub;
uint8 fRootHubAddress;
@@ -267,8 +264,16 @@ private:
struct xhci_device fDevices[XHCI_MAX_DEVICES];
int32 fContextSizeShift; //
0/1 for 32/64 bytes
+ // Transfers
+ mutex fFinishedLock;
+ xhci_td * fFinishedHead;
+ sem_id fFinishTransfersSem;
+ thread_id fFinishThread;
+
+ // Events
sem_id fEventSem;
thread_id fEventThread;
+ mutex fEventLock;
uint16 fEventIdx;
uint16 fCmdIdx;
uint8 fEventCcs;
