Re: [RFC 1/1] usb: host: xhci-plat: add enable XHCI-AVOID-BEI quirk by dts

From: Mathias Nyman
Date: Mon Jun 03 2024 - 08:22:32 EST


On 1.6.2024 15.06, joswang wrote:
From: joswang <joswang@xxxxxxxxxx>

For Synopsys DWC31 2.00a and earlier versions, every isochronous
interval the BEI(Block Event Interrupt) flag is set for all except
the last Isoch TRB in a URB, host controller consumes the event
TRBs in the event ring, once the event ring is full, it will not
generate an interrupt and will stop all data transmission and command
execution.

To avoid the problem of event ring full, the XHCI-AVOID-BEI quirk is
introduced. Currently, the XHCI-AVOID-BEI quirk has been applied to all
Intel xHCI controllers, see commit '227a4fd801c8 ("USB: xhci: apply
XHCI-AVOID-BEI quirk to all Intel xHCI controllers")'.

For Linux system, each event ring consists of one or more event ring
segments and each segment is 4 KB that contains 256 TRBs. It seems that
the TRBs on the event ring are sufficient and the event ring will not be
full. In real application, if it does happen, event ring is full, host
controller no interrupt causes the driver to timeout.

However, applying XHCI-AVOID-BEI quirk will also bring power consumption
issues. We can consider the application scenarios of the product to decide
whether to enable it. Therefore, we add the enable XHCI-AVOID-BEI quirk
through dts configuration to make it more flexible.

Took a look at XHCI_AVOID_BEI quirk and it seems that it evolved from
solving a hardware issue into a interrupt trigger optimization.

How about making current XHCI_AVOID_BEI the default behavior, i.e. force
an interrupt every 32nd isoc trb, and reduce it in case event ring
has more than half a segments of events per interrupt.

The actual XHCI_AVOID_BEI quirk would only be used for hardware that that
can't handle BEI flag properly

something like:

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 266fcbc4bb93..dd161ebf15a3 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -3991,16 +3991,17 @@ static int xhci_get_isoc_frame_id(struct xhci_hcd *xhci,
static bool trb_block_event_intr(struct xhci_hcd *xhci, int num_tds, int i,
struct xhci_interrupter *ir)
{
- if (xhci->hci_version < 0x100)
+ if (xhci->hci_version < 0x100 || xhci->quirks & XHCI_AVOID_BEI)
return false;
+
/* always generate an event interrupt for the last TD */
if (i == num_tds - 1)
return false;
/*
- * If AVOID_BEI is set the host handles full event rings poorly,
- * generate an event at least every 8th TD to clear the event ring
+ * host handles full event rings poorly, force an interrupt at least
+ * every 32 isoc TRB to clear the event ring.
*/
- if (i && ir->isoc_bei_interval && xhci->quirks & XHCI_AVOID_BEI)
+ if (i && ir->isoc_bei_interval)


Thanks
Mathias