Received-SPF: pass (google.com: domain of linux-kernel+bounces-199104-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Message-ID: <1662c9c9-7330-4794-843a-940f05802021@linux.intel.com>
Date: Mon, 3 Jun 2024 15:22:32 +0300
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Firefox/102.0 Thunderbird/102.13.0
Content-Language: en-US
To: joswang <joswang1221@gmail.com>, mathias.nyman@intel.com,
 gregkh@linuxfoundation.org
Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
 joswang <joswang@lenovo.com>
References: <20240601120640.73556-1-joswang1221@gmail.com>
From: Mathias Nyman <mathias.nyman@linux.intel.com>
Subject: Re: [RFC 1/1] usb: host: xhci-plat: add enable XHCI-AVOID-BEI quirk
 by dts
In-Reply-To: <20240601120640.73556-1-joswang1221@gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

On 1.6.2024 15.06, joswang wrote:
> From: joswang <joswang@lenovo.com>
> 
> 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