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

[Mathias Nyman]

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